Skip to contentSkip to conversation

qcom: Add support for GDSCs

PATCH New Rajendra Nayak 2 participants 30 messages Aug 6, 2015 – Dec 1, 2015

Subsystems (5)
Device tree bindings Arm Qualcomm mailing list Common clk Arm Qualcomm support +1 more subsystems
lore
Share & tools
lore

Rajendra Nayak

author
#0 lore raw

v8 cover qcom: Add support for GDSCs

GDSCs (Global Distributed Switch Controllers) control switches that supply power to an on-chip power domain and hence can be programmed in SW to safely power collapse and restore power to the respective PDs. They are part of a considerable number of recent QCOM SoCs (This series adds support for msm8974, msm8916 and apq8084 devices) and are part of the Clock control block.

The series implements support for GDSC using the genpd framework modelling these as SW controllable power domains and uses PM clocks to control clocks within the gdsc.

* Client drivers which plan to use GDSC can refer to Documentation/devicetree/bindings/power/power_domain.txt to know how to hook up the power domain for the device through DT
* Runtime PM specific documentation can be found in Documentation/power/runtime_pm.txt
* Patches are based of 4.2-rc5

Tested with and without CONFIG_PM on db8074, IFC6540 and db410c (arm64)

Changes since v7:
* used of_clk_get_from_provider() instead of a con_id based clk_get() This is based on an offlist discussion with Stephen.
* Moved the clock control patches to the end of the series, to enable the rest of the gdsc support patches (1 to 10) to get merged if clock control needs more discussions
* Added missing gdscs' from mmcc apq8084 (pointed to by Stan)

Changes since v6:
* removed hardcoded con_ids, now part of gdsc struct
* other minor review fixes

Changes since v5:
* Added support to manage clocks with !CONFIG_PM
* Added RET/OFF support
* Added support to assert/deassert reset while the GDSC is ON

Changes since v4:
* Added clock control support using PM clocks
* Included WA to enable an RCG (needed for Oxili gdsc in 8916)

Changes since v3:
* static inline'd gdsc_register/unregister stubs
* error check fixes in gdsc_register
* dropped oxili_gdsc for 8916 as its broken and needs additional WA's not part of this series
* split dts and driver changes into seperate patches

Changes since v2:
* gdsc_unregister added
* gdsc_register/unregister introduced in patch 1/6

Changes since v1:
* added err checks for regmap apis
* added gdsc_register() in gdsc.c

Rajendra Nayak (9):
clk: qcom: gdsc: Prepare common clk probe to register gdscs
clk: qcom: gdsc: Add support for Memory RET/OFF
clk: qcom: gdsc: Add support for ON only state
clk: qcom: gdsc: Add GDSCs in msm8916 GCC
clk: qcom: gdsc: Add GDSCs in apq8084 GCC
arm: dts: qcom: Add #power-domain-cells property
clk: qcom: gdsc: Use PM clocks to control gdsc clocks
clk: qcom: gdsc: Enable an RCG before turing on the gdsc
clk: qcom: gdsc: Manage clocks with !CONFIG_PM

Stephane Viau (1):
clk: qcom: gdsc: Add GDSCs in apq8084 MMCC

Stephen Boyd (3):
clk: qcom: Add support for GDSCs
clk: qcom: gdsc: Add GDSCs in msm8974 GCC
clk: qcom: gdsc: Add GDSCs in msm8974 MMCC

 arch/arm/boot/dts/qcom-apq8084.dtsi           |   1 +
 arch/arm/boot/dts/qcom-msm8974.dtsi           |   2 +
 arch/arm64/boot/dts/qcom/msm8916.dtsi         |   1 +
 drivers/clk/qcom/Kconfig                      |   9 +
 drivers/clk/qcom/Makefile                     |   1 +
 drivers/clk/qcom/common.c                     |  16 +-
 drivers/clk/qcom/common.h                     |   2 +
 drivers/clk/qcom/gcc-apq8084.c                |  42 +++
 drivers/clk/qcom/gcc-msm8916.c                |  52 ++++
 drivers/clk/qcom/gcc-msm8974.c                |  15 +
 drivers/clk/qcom/gdsc.c                       | 379 ++++++++++++++++++++++++++
 drivers/clk/qcom/gdsc.h                       |  87 ++++++
 drivers/clk/qcom/mmcc-apq8084.c               |  86 +++++-
 drivers/clk/qcom/mmcc-msm8974.c               |  72 +++++
 include/dt-bindings/clock/qcom,gcc-apq8084.h  |   6 +
 include/dt-bindings/clock/qcom,gcc-msm8916.h  |   8 +
 include/dt-bindings/clock/qcom,gcc-msm8974.h  |   3 +
 include/dt-bindings/clock/qcom,mmcc-apq8084.h |  10 +
 include/dt-bindings/clock/qcom,mmcc-msm8974.h |   8 +
 19 files changed, 798 insertions(+), 2 deletions(-)
 create mode 100644 drivers/clk/qcom/gdsc.c
 create mode 100644 drivers/clk/qcom/gdsc.h
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
author
#1 lore raw

PATCH v8 1/13 clk: qcom: Add support for GDSCs

From: Stephen Boyd <[email protected]>

GDSCs (Global Distributed Switch Controllers) are responsible for safely collapsing and restoring power to peripherals in the SoC. These are best modelled as power domains using genpd and given the registers are scattered throughout the clock controller register space, its best to have the support added through the clock driver.

Signed-off-by Stephen Boyd <[email protected]>
Signed-off-by Rajendra Nayak <[email protected]>
---
 drivers/clk/qcom/Kconfig  |   4 ++
 drivers/clk/qcom/Makefile |   1 +
 drivers/clk/qcom/gdsc.c   | 171 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/gdsc.h   |  46 +++++++++++++
 4 files changed, 222 insertions(+)
 create mode 100644 drivers/clk/qcom/gdsc.c
 create mode 100644 drivers/clk/qcom/gdsc.h
drivers/clk/qcom/Kconfig +4 -0
index 59d1666..d3a695b 100644
@@ -39,6 +39,10 @@ config IPQ_LCC_806X
 	  Say Y if you want to use audio devices such as i2s, pcm,
 	  S/PDIF, etc.
 
+config QCOM_GDSC
+	bool
+	select PM_GENERIC_DOMAINS if PM
+
 config MSM_GCC_8660
 	tristate "MSM8660 Global Clock Controller"
 	depends on COMMON_CLK_QCOM
drivers/clk/qcom/Makefile +1 -0
index 50b337a..fe62523 100644
@@ -9,6 +9,7 @@ clk-qcom-y += clk-branch.o
 clk-qcom-y += clk-regmap-divider.o
 clk-qcom-y += clk-regmap-mux.o
 clk-qcom-y += reset.o
+clk-qcom-$(CONFIG_QCOM_GDSC) += gdsc.o
 
 obj-$(CONFIG_APQ_GCC_8084) += gcc-apq8084.o
 obj-$(CONFIG_APQ_MMCC_8084) += mmcc-apq8084.o
drivers/clk/qcom/gdsc.c new file +171 -0

171 lines not on this page · raw message

drivers/clk/qcom/gdsc.h new file +46 -0

46 lines not on this page · raw message

Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Applied
#2 lore raw

PATCH v8 1/13 clk: qcom: Add support for GDSCs

Quoted 11 lines · Rajendra Nayak

From: Stephen Boyd <[email protected]>

GDSCs (Global Distributed Switch Controllers) are responsible for safely collapsing and restoring power to peripherals in the SoC. These are best modelled as power domains using genpd and given the registers are scattered throughout the clock controller register space, its best to have the support added through the clock driver.

Signed-off-by Stephen Boyd <[email protected]>
Signed-off-by Rajendra Nayak <[email protected]>

---

Applied to clk-next

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#3 lore raw

PATCH v8 2/13 clk: qcom: gdsc: Prepare common clk probe to register gdscs

In reply to Rajendra Nayak

The common clk probe registers a clk provider and a reset controller. Update it to register a genpd provider using the gdsc data provided by each platform.

Signed-off-by Rajendra Nayak <[email protected]>
---
 drivers/clk/qcom/common.c | 15 ++++++++++++++-
 drivers/clk/qcom/common.h |  2 ++
 2 files changed, 16 insertions(+), 1 deletion(-)
drivers/clk/qcom/common.c +14 -1
index f7101e3..a319aa8 100644
@@ -21,6 +21,7 @@
 #include "clk-rcg.h"
 #include "clk-regmap.h"
 #include "reset.h"
+#include "gdsc.h"
 
 struct qcom_cc {
 	struct qcom_reset_controller reset;

@@ -120,8 +121,19 @@ int qcom_cc_really_probe(struct platform_device *pdev,
 
 	ret = reset_controller_register(&reset->rcdev);
 	if (ret)
-		of_clk_del_provider(dev->of_node);
+		goto err_reset;
 
+	if (desc->gdscs && desc->num_gdscs) {
+		ret = gdsc_register(dev, desc->gdscs, desc->num_gdscs, regmap);
+		if (ret)
+			goto err_pd;
+	}
+
+	return 0;
+err_pd:
+	reset_controller_unregister(&reset->rcdev);
+err_reset:
+	of_clk_del_provider(dev->of_node);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(qcom_cc_really_probe);

@@ -140,6 +152,7 @@ EXPORT_SYMBOL_GPL(qcom_cc_probe);
 
 void qcom_cc_remove(struct platform_device *pdev)
 {
+	gdsc_unregister(&pdev->dev);
 	of_clk_del_provider(pdev->dev.of_node);
 	reset_controller_unregister(platform_get_drvdata(pdev));
 }
drivers/clk/qcom/common.h +2 -0
index 7a0e737..2892b71f 100644
@@ -28,6 +28,8 @@ struct qcom_cc_desc {
 	size_t num_clks;
 	const struct qcom_reset_map *resets;
 	size_t num_resets;
+	struct gdsc **gdscs;
+	size_t num_gdscs;
 };
 
 extern const struct freq_tbl *qcom_find_freq(const struct freq_tbl *f,
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Applied
#4 lore raw

PATCH v8 2/13 clk: qcom: gdsc: Prepare common clk probe to register gdscs

Quoted 6 lines · Rajendra Nayak

The common clk probe registers a clk provider and a reset controller. Update it to register a genpd provider using the gdsc data provided by each platform.

Signed-off-by Rajendra Nayak <[email protected]>

---

Applied to clk-next

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#5 lore raw

PATCH v8 3/13 clk: qcom: gdsc: Add support for Memory RET/OFF

In reply to Rajendra Nayak

Along with the GDSC power switch, there is additional control to either retain all memory (core and peripheral) within a given powerdomain or to turn them off while the GDSC is powered down. Add support for these by modelling a RET state where all
memory is retained and an OFF state where all memory gets turned off.
The controls provided are granular enough to be able to support various differnt levels of RET states, like a 'shallow RET' with all memory retained and a 'deep RET' with some memory retained while some others are lost. The current patch does not support this and considers just one RET state where all memory is retained. Futher work, if needed can support multiple different levels of RET state.

Signed-off-by Rajendra Nayak <[email protected]>
---
 drivers/clk/qcom/gdsc.c | 33 +++++++++++++++++++++++++++++++++
 drivers/clk/qcom/gdsc.h | 13 +++++++++++++
 2 files changed, 46 insertions(+)
drivers/clk/qcom/gdsc.c +33 -0
index 469b4c4..e6bbb76 100644
@@ -34,6 +34,9 @@
 #define EN_FEW_WAIT_VAL		(0x8 << 16)
 #define CLK_DIS_WAIT_VAL	(0x2 << 12)
 
+#define RETAIN_MEM		BIT(14)
+#define RETAIN_PERIPH		BIT(13)
+
 #define TIMEOUT_US		100
 
 #define domain_to_gdsc(domain) container_of(domain, struct gdsc, pd)

@@ -81,6 +84,24 @@ static int gdsc_toggle_logic(struct gdsc *sc, bool en)
 	return -ETIMEDOUT;
 }
 
+static inline void gdsc_force_mem_on(struct gdsc *sc)
+{
+	int i;
+	u32 mask = RETAIN_MEM | RETAIN_PERIPH;
+
+	for (i = 0; i < sc->cxc_count; i++)
+		regmap_update_bits(sc->regmap, sc->cxcs[i], mask, mask);
+}
+
+static inline void gdsc_clear_mem_on(struct gdsc *sc)
+{
+	int i;
+	u32 mask = RETAIN_MEM | RETAIN_PERIPH;
+
+	for (i = 0; i < sc->cxc_count; i++)
+		regmap_update_bits(sc->regmap, sc->cxcs[i], mask, 0);
+}
+
 static int gdsc_enable(struct generic_pm_domain *domain)
 {
 	struct gdsc *sc = domain_to_gdsc(domain);

@@ -89,6 +110,10 @@ static int gdsc_enable(struct generic_pm_domain *domain)
 	ret = gdsc_toggle_logic(sc, true);
 	if (ret)
 		return ret;
+
+	if (sc->pwrsts & PWRSTS_OFF)
+		gdsc_force_mem_on(sc);
+
 	/*
 	 * If clocks to this power domain were already on, they will take an
 	 * additional 4 clock cycles to re-enable after the power domain is

@@ -105,6 +130,9 @@ static int gdsc_disable(struct generic_pm_domain *domain)
 {
 	struct gdsc *sc = domain_to_gdsc(domain);
 
+	if (sc->pwrsts & PWRSTS_OFF)
+		gdsc_clear_mem_on(sc);
+
 	return gdsc_toggle_logic(sc, false);
 }
 

@@ -129,6 +157,11 @@ static int gdsc_init(struct gdsc *sc)
 	if (on < 0)
 		return on;
 
+	if (on || (sc->pwrsts & PWRSTS_RET))
+		gdsc_force_mem_on(sc);
+	else
+		gdsc_clear_mem_on(sc);
+
 	sc->pd.power_off = gdsc_disable;
 	sc->pd.power_on = gdsc_enable;
 	pm_genpd_init(&sc->pd, NULL, !on);
drivers/clk/qcom/gdsc.h +13 -0
index f578a0c..0ff251a 100644
@@ -19,16 +19,29 @@
 
 struct regmap;
 
+/* Powerdomain allowable state bitfields */
+#define PWRSTS_OFF		BIT(0)
+#define PWRSTS_RET		BIT(1)
+#define PWRSTS_ON		BIT(2)
+#define PWRSTS_OFF_ON		(PWRSTS_OFF | PWRSTS_ON)
+#define PWRSTS_RET_ON		(PWRSTS_RET | PWRSTS_ON)
+
 /**
  * struct gdsc - Globally Distributed Switch Controller
  * @pd: generic power domain
  * @regmap: regmap for MMIO accesses
  * @gdscr: gsdc control register
+ * @cxcs: offsets of branch registers to toggle mem/periph bits in
+ * @cxc_count: number of @cxcs
+ * @pwrsts: Possible powerdomain power states
  */
 struct gdsc {
 	struct generic_pm_domain	pd;
 	struct regmap			*regmap;
 	unsigned int			gdscr;
+	unsigned int			*cxcs;
+	unsigned int			cxc_count;
+	const u8			pwrsts;
 };
 
 #ifdef CONFIG_QCOM_GDSC
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Applied
#6 lore raw

PATCH v8 3/13 clk: qcom: gdsc: Add support for Memory RET/OFF

Quoted 13 lines · Rajendra Nayak

Along with the GDSC power switch, there is additional control to either retain all memory (core and peripheral) within a given powerdomain or to turn them off while the GDSC is powered down. Add support for these by modelling a RET state where all
memory is retained and an OFF state where all memory gets turned off.
The controls provided are granular enough to be able to support various differnt levels of RET states, like a 'shallow RET' with all memory retained and a 'deep RET' with some memory retained while some others are lost. The current patch does not support this and considers just one RET state where all memory is retained. Futher work, if needed can support multiple different levels of RET state.

Quoted 2 lines
Signed-off-by Rajendra Nayak <[email protected]>

---

Applied to clk-next

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#7 lore raw

PATCH v8 4/13 clk: qcom: gdsc: Add support for ON only state

In reply to Rajendra Nayak

Certain devices can have GDSCs' which support ON as the only state. They can't be power collapsed to either hit RET or OFF. The clients drivers for these GDSCs' however would expect the state of the core to be reset following a GDSC disable and re-enable. To do this assert/deassert reset lines every time the client driver would request the GDSC to be powered on/off instead.

Signed-off-by Rajendra Nayak <[email protected]>
---
 drivers/clk/qcom/common.c |  3 ++-
 drivers/clk/qcom/gdsc.c   | 35 ++++++++++++++++++++++++++++++++++-
 drivers/clk/qcom/gdsc.h   | 11 ++++++++++-
 3 files changed, 46 insertions(+), 3 deletions(-)
drivers/clk/qcom/common.c +2 -1
index a319aa8..4bc87e1 100644
@@ -124,7 +124,8 @@ int qcom_cc_really_probe(struct platform_device *pdev,
 		goto err_reset;
 
 	if (desc->gdscs && desc->num_gdscs) {
-		ret = gdsc_register(dev, desc->gdscs, desc->num_gdscs, regmap);
+		ret = gdsc_register(dev, desc->gdscs, desc->num_gdscs,
+				    &reset->rcdev, regmap);
 		if (ret)
 			goto err_pd;
 	}
drivers/clk/qcom/gdsc.c +34 -1
index e6bbb76..da9fad8 100644
@@ -18,6 +18,7 @@
 #include <linux/kernel.h>
 #include <linux/pm_domain.h>
 #include <linux/regmap.h>
+#include <linux/reset-controller.h>
 #include <linux/slab.h>
 #include "gdsc.h"
 

@@ -84,6 +85,24 @@ static int gdsc_toggle_logic(struct gdsc *sc, bool en)
 	return -ETIMEDOUT;
 }
 
+static inline int gdsc_deassert_reset(struct gdsc *sc)
+{
+	int i;
+
+	for (i = 0; i < sc->reset_count; i++)
+		sc->rcdev->ops->deassert(sc->rcdev, sc->resets[i]);
+	return 0;
+}
+
+static inline int gdsc_assert_reset(struct gdsc *sc)
+{
+	int i;
+
+	for (i = 0; i < sc->reset_count; i++)
+		sc->rcdev->ops->assert(sc->rcdev, sc->resets[i]);
+	return 0;
+}
+
 static inline void gdsc_force_mem_on(struct gdsc *sc)
 {
 	int i;

@@ -107,6 +126,9 @@ static int gdsc_enable(struct generic_pm_domain *domain)
 	struct gdsc *sc = domain_to_gdsc(domain);
 	int ret;
 
+	if (sc->pwrsts == PWRSTS_ON)
+		return gdsc_deassert_reset(sc);
+
 	ret = gdsc_toggle_logic(sc, true);
 	if (ret)
 		return ret;

@@ -130,6 +152,9 @@ static int gdsc_disable(struct generic_pm_domain *domain)
 {
 	struct gdsc *sc = domain_to_gdsc(domain);
 
+	if (sc->pwrsts == PWRSTS_ON)
+		return gdsc_assert_reset(sc);
+
 	if (sc->pwrsts & PWRSTS_OFF)
 		gdsc_clear_mem_on(sc);
 

@@ -153,6 +178,13 @@ static int gdsc_init(struct gdsc *sc)
 	if (ret)
 		return ret;
 
+	/* Force gdsc ON if only ON state is supported */
+	if (sc->pwrsts == PWRSTS_ON) {
+		ret = gdsc_toggle_logic(sc, true);
+		if (ret)
+			return ret;
+	}
+
 	on = gdsc_is_enabled(sc);
 	if (on < 0)
 		return on;

@@ -170,7 +202,7 @@ static int gdsc_init(struct gdsc *sc)
 }
 
 int gdsc_register(struct device *dev, struct gdsc **scs, size_t num,
-		  struct regmap *regmap)
+		  struct reset_controller_dev *rcdev, struct regmap *regmap)
 {
 	int i, ret;
 	struct genpd_onecell_data *data;

@@ -189,6 +221,7 @@ int gdsc_register(struct device *dev, struct gdsc **scs, size_t num,
 		if (!scs[i])
 			continue;
 		scs[i]->regmap = regmap;
+		scs[i]->rcdev = rcdev;
 		ret = gdsc_init(scs[i]);
 		if (ret)
 			return ret;
drivers/clk/qcom/gdsc.h +10 -1
index 0ff251a..5ded268 100644
@@ -18,6 +18,7 @@
 #include <linux/pm_domain.h>
 
 struct regmap;
+struct reset_controller_dev;
 
 /* Powerdomain allowable state bitfields */
 #define PWRSTS_OFF		BIT(0)

@@ -34,6 +35,9 @@ struct regmap;
  * @cxcs: offsets of branch registers to toggle mem/periph bits in
  * @cxc_count: number of @cxcs
  * @pwrsts: Possible powerdomain power states
+ * @resets: ids of resets associated with this gdsc
+ * @reset_count: number of @resets
+ * @rcdev: reset controller
  */
 struct gdsc {
 	struct generic_pm_domain	pd;

@@ -42,13 +46,18 @@ struct gdsc {
 	unsigned int			*cxcs;
 	unsigned int			cxc_count;
 	const u8			pwrsts;
+	struct reset_controller_dev	*rcdev;
+	unsigned int			*resets;
+	unsigned int			reset_count;
 };
 
 #ifdef CONFIG_QCOM_GDSC
-int gdsc_register(struct device *, struct gdsc **, size_t n, struct regmap *);
+int gdsc_register(struct device *, struct gdsc **, size_t n,
+		  struct reset_controller_dev *, struct regmap *);
 void gdsc_unregister(struct device *);
 #else
 static inline int gdsc_register(struct device *d, struct gdsc **g, size_t n,
+				struct reset_controller_dev *rcdev,
 				struct regmap *r)
 {
 	return -ENOSYS;
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Applied
#8 lore raw

PATCH v8 4/13 clk: qcom: gdsc: Add support for ON only state

Quoted 9 lines · Rajendra Nayak

Certain devices can have GDSCs' which support ON as the only state. They can't be power collapsed to either hit RET or OFF. The clients drivers for these GDSCs' however would expect the state of the core to be reset following a GDSC disable and re-enable. To do this assert/deassert reset lines every time the client driver would request the GDSC to be powered on/off instead.

Signed-off-by Rajendra Nayak <[email protected]>

---

Applied to clk-next

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#9 lore raw

PATCH v8 5/13 clk: qcom: gdsc: Add GDSCs in msm8916 GCC

In reply to Rajendra Nayak

Add all data for the GDSCs which are part of msm8916 GCC block.

Signed-off-by Rajendra Nayak <[email protected]>
---
 drivers/clk/qcom/Kconfig                     |  1 +
 drivers/clk/qcom/gcc-msm8916.c               | 51 ++++++++++++++++++++++++++++
 include/dt-bindings/clock/qcom,gcc-msm8916.h |  8 +++++
 3 files changed, 60 insertions(+)
drivers/clk/qcom/Kconfig +1 -0
index d3a695b..abec45a 100644
@@ -53,6 +53,7 @@ config MSM_GCC_8660
 
 config MSM_GCC_8916
 	tristate "MSM8916 Global Clock Controller"
+	select QCOM_GDSC
 	depends on COMMON_CLK_QCOM
 	help
 	  Support for the global clock controller on msm8916 devices.
drivers/clk/qcom/gcc-msm8916.c +51 -0

75 lines not on this page · raw message

include/dt-bindings/clock/qcom,gcc-msm8916.h +8 -0
index e430f64..11566c5 100644
@@ -153,4 +153,12 @@
 #define GCC_VENUS0_AXI_CLK			136
 #define GCC_VENUS0_VCODEC0_CLK			137
 
+/* Indexes for GDSCs */
+#define BIMC_GDSC				0
+#define VENUS_GDSC				1
+#define MDSS_GDSC				2
+#define JPEG_GDSC				3
+#define VFE_GDSC				4
+#define OXILI_GDSC				5
+
 #endif
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Applied
#10 lore raw

PATCH v8 5/13 clk: qcom: gdsc: Add GDSCs in msm8916 GCC

Quoted 4 lines · Rajendra Nayak

Add all data for the GDSCs which are part of msm8916 GCC block.

Signed-off-by Rajendra Nayak <[email protected]>

---

Applied to clk-next

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#11 lore raw

PATCH v8 6/13 clk: qcom: gdsc: Add GDSCs in msm8974 GCC

In reply to Rajendra Nayak

From: Stephen Boyd <[email protected]>

There's just one GDSC as part of the msm8974 GCC block.

Signed-off-by Stephen Boyd <[email protected]>
Signed-off-by Rajendra Nayak <[email protected]>
---
 drivers/clk/qcom/Kconfig                     |  1 +
 drivers/clk/qcom/gcc-msm8974.c               | 15 +++++++++++++++
 include/dt-bindings/clock/qcom,gcc-msm8974.h |  3 +++
 3 files changed, 19 insertions(+)
drivers/clk/qcom/Kconfig +1 -0
index abec45a..a6fcb1d 100644
@@ -88,6 +88,7 @@ config MSM_MMCC_8960
 
 config MSM_GCC_8974
 	tristate "MSM8974 Global Clock Controller"
+	select QCOM_GDSC
 	depends on COMMON_CLK_QCOM
 	help
 	  Support for the global clock controller on msm8974 devices.
drivers/clk/qcom/gcc-msm8974.c +15 -0
index c39d098..2c3c26f 100644
@@ -31,6 +31,7 @@
 #include "clk-rcg.h"
 #include "clk-branch.h"
 #include "reset.h"
+#include "gdsc.h"
 
 enum {
 	P_XO,

@@ -2431,6 +2432,14 @@ static struct clk_branch gcc_usb_hsic_system_clk = {
 	},
 };
 
+static struct gdsc usb_hs_hsic_gdsc = {
+	.gdscr = 0x404,
+	.pd = {
+		.name = "usb_hs_hsic",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+};
+
 static struct clk_regmap *gcc_msm8974_clocks[] = {
 	[GPLL0] = &gpll0.clkr,
 	[GPLL0_VOTE] = &gpll0_vote,

@@ -2660,6 +2669,10 @@ static const struct qcom_reset_map gcc_msm8974_resets[] = {
 	[GCC_VENUS_RESTART] = { 0x1740 },
 };
 
+static struct gdsc *gcc_msm8974_gdscs[] = {
+	[USB_HS_HSIC_GDSC] = &usb_hs_hsic_gdsc,
+};
+
 static const struct regmap_config gcc_msm8974_regmap_config = {
 	.reg_bits	= 32,
 	.reg_stride	= 4,

@@ -2674,6 +2687,8 @@ static const struct qcom_cc_desc gcc_msm8974_desc = {
 	.num_clks = ARRAY_SIZE(gcc_msm8974_clocks),
 	.resets = gcc_msm8974_resets,
 	.num_resets = ARRAY_SIZE(gcc_msm8974_resets),
+	.gdscs = gcc_msm8974_gdscs,
+	.num_gdscs = ARRAY_SIZE(gcc_msm8974_gdscs),
 };
 
 static const struct of_device_id gcc_msm8974_match_table[] = {
include/dt-bindings/clock/qcom,gcc-msm8974.h +3 -0
index 51e51c8..81d32f6 100644
@@ -321,4 +321,7 @@
 #define GCC_SDCC1_CDCCAL_SLEEP_CLK				304
 #define GCC_SDCC1_CDCCAL_FF_CLK					305
 
+/* gdscs */
+#define USB_HS_HSIC_GDSC					0
+
 #endif
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
author
#13 lore raw

PATCH v8 7/13 clk: qcom: gdsc: Add GDSCs in msm8974 MMCC

In reply to Rajendra Nayak

From: Stephen Boyd <[email protected]>

Add the GDSC instances that exist as part of msm8974 MMCC block

Signed-off-by Stephen Boyd <[email protected]>
Signed-off-by Rajendra Nayak <[email protected]>
---
 drivers/clk/qcom/Kconfig                      |  1 +
 drivers/clk/qcom/mmcc-msm8974.c               | 72 +++++++++++++++++++++++++++
 include/dt-bindings/clock/qcom,mmcc-msm8974.h |  8 +++
 3 files changed, 81 insertions(+)
drivers/clk/qcom/Kconfig +1 -0
index a6fcb1d..edab172 100644
@@ -98,6 +98,7 @@ config MSM_GCC_8974
 config MSM_MMCC_8974
 	tristate "MSM8974 Multimedia Clock Controller"
 	select MSM_GCC_8974
+	select QCOM_GDSC
 	depends on COMMON_CLK_QCOM
 	help
 	  Support for the multimedia clock controller on msm8974 devices.
drivers/clk/qcom/mmcc-msm8974.c +72 -0

96 lines not on this page · raw message

include/dt-bindings/clock/qcom,mmcc-msm8974.h +8 -0
index 032ed87..28651e5 100644
@@ -158,4 +158,12 @@
 #define SPDM_RM_AXI					141
 #define SPDM_RM_OCMEMNOC				142
 
+/* gdscs */
+#define VENUS0_GDSC					0
+#define MDSS_GDSC					1
+#define CAMSS_JPEG_GDSC					2
+#define CAMSS_VFE_GDSC					3
+#define OXILI_GDSC					4
+#define OXILICX_GDSC					5
+
 #endif
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
author
#15 lore raw

PATCH v8 8/13 clk: qcom: gdsc: Add GDSCs in apq8084 GCC

In reply to Rajendra Nayak

Add the GDSC instances that exist as part of apq8084 GCC block

Signed-off-by Rajendra Nayak <[email protected]>
---
 drivers/clk/qcom/Kconfig                     |  1 +
 drivers/clk/qcom/gcc-apq8084.c               | 42 ++++++++++++++++++++++++++++
 include/dt-bindings/clock/qcom,gcc-apq8084.h |  6 ++++
 3 files changed, 49 insertions(+)
drivers/clk/qcom/Kconfig +1 -0
index edab172..fe00dd6 100644
@@ -7,6 +7,7 @@ config COMMON_CLK_QCOM
 
 config APQ_GCC_8084
 	tristate "APQ8084 Global Clock Controller"
+	select QCOM_GDSC
 	depends on COMMON_CLK_QCOM
 	help
 	  Support for the global clock controller on apq8084 devices.
drivers/clk/qcom/gcc-apq8084.c +42 -0

66 lines not on this page · raw message

include/dt-bindings/clock/qcom,gcc-apq8084.h +6 -0
index 2c0da56..5aa7ebe 100644
@@ -348,4 +348,10 @@
 #define GCC_PCIE_1_PIPE_CLK				331
 #define GCC_PCIE_1_SLV_AXI_CLK				332
 
+/* gdscs */
+#define USB_HS_HSIC_GDSC				0
+#define PCIE0_GDSC					1
+#define PCIE1_GDSC					2
+#define USB30_GDSC					3
+
 #endif
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Applied
#16 lore raw

PATCH v8 8/13 clk: qcom: gdsc: Add GDSCs in apq8084 GCC

Quoted 4 lines · Rajendra Nayak

Add the GDSC instances that exist as part of apq8084 GCC block

Signed-off-by Rajendra Nayak <[email protected]>

---

Applied to clk-next

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#17 lore raw

PATCH v8 9/13 clk: qcom: gdsc: Add GDSCs in apq8084 MMCC

In reply to Rajendra Nayak

From: Stephane Viau <[email protected]>

Add the GDSC instances that exist as part of apq8084 MMCC block.

Signed-off-by Stephane Viau <[email protected]>
Signed-off-by Rajendra Nayak <[email protected]>
---
 drivers/clk/qcom/Kconfig                      |  1 +
 drivers/clk/qcom/mmcc-apq8084.c               | 86 ++++++++++++++++++++++++++-
 include/dt-bindings/clock/qcom,mmcc-apq8084.h | 10 ++++
 3 files changed, 96 insertions(+), 1 deletion(-)
drivers/clk/qcom/Kconfig +1 -0
index fe00dd6..47b988f 100644
@@ -17,6 +17,7 @@ config APQ_GCC_8084
 config APQ_MMCC_8084
 	tristate "APQ8084 Multimedia Clock Controller"
 	select APQ_GCC_8084
+	select QCOM_GDSC
 	depends on COMMON_CLK_QCOM
 	help
 	  Support for the multimedia clock controller on apq8084 devices.
drivers/clk/qcom/mmcc-apq8084.c +85 -1

114 lines not on this page · raw message

include/dt-bindings/clock/qcom,mmcc-apq8084.h +10 -0
index d72b5b3..03861e3 100644
@@ -180,4 +180,14 @@
 #define VPU_SLEEP_CLK			163
 #define VPU_VDP_CLK			164
 
+/* GDSCs */
+#define VENUS0_GDSC			0
+#define VENUS0_CORE0_GDSC		1
+#define VENUS0_CORE1_GDSC		2
+#define MDSS_GDSC			3
+#define CAMSS_JPEG_GDSC			4
+#define CAMSS_VFE_GDSC			5
+#define OXILI_GDSC			6
+#define OXILICX_GDSC			7
+
 #endif
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
author
#19 lore raw

PATCH v8 10/13 arm: dts: qcom: Add #power-domain-cells property

In reply to Rajendra Nayak

msm8974 has gcc and mmcc nodes, and apq8084 has a gcc node which implement gdsc powerdomains. Add the #power-domain-cells property to them.

Signed-off-by Rajendra Nayak <[email protected]>
---
 arch/arm/boot/dts/qcom-apq8084.dtsi   | 1 +
 arch/arm/boot/dts/qcom-msm8974.dtsi   | 2 ++
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 1 +
 3 files changed, 4 insertions(+)
arch/arm/boot/dts/qcom-apq8084.dtsi +1 -0
index 7084010..1cdb568 100644
@@ -221,6 +221,7 @@
 			compatible = "qcom,gcc-apq8084";
 			#clock-cells = <1>;
 			#reset-cells = <1>;
+			#power-domain-cells = <1>;
 			reg = <0xfc400000 0x4000>;
 		};
arch/arm/boot/dts/qcom-msm8974.dtsi +2 -0
index 37b47b5..8a2dfa6 100644
@@ -217,6 +217,7 @@
 			compatible = "qcom,gcc-msm8974";
 			#clock-cells = <1>;
 			#reset-cells = <1>;
+			#power-domain-cells = <1>;
 			reg = <0xfc400000 0x4000>;
 		};
 

@@ -224,6 +225,7 @@
 			compatible = "qcom,mmcc-msm8974";
 			#clock-cells = <1>;
 			#reset-cells = <1>;
+			#power-domain-cells = <1>;
 			reg = <0xfd8c0000 0x6000>;
 		};
arch/arm64/boot/dts/qcom/msm8916.dtsi +1 -0
index 0f49ebd..3b0645a 100644
@@ -120,6 +120,7 @@
 			compatible = "qcom,gcc-msm8916";
 			#clock-cells = <1>;
 			#reset-cells = <1>;
+			#power-domain-cells = <1>;
 			reg = <0x1800000 0x80000>;
 		};
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#20 lore raw

PATCH v8 10/13 arm: dts: qcom: Add #power-domain-cells property

Quoted 6 lines · Rajendra Nayak

msm8974 has gcc and mmcc nodes, and apq8084 has a gcc node which implement gdsc powerdomains. Add the #power-domain-cells property to them.

Signed-off-by Rajendra Nayak <[email protected]>

---

This needs to go through Andy's tree and arm-soc. Also, I don't see an update for the binding anywhere. Please update the binding too.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#21 lore raw

PATCH v8 10/13 arm: dts: qcom: Add #power-domain-cells property

Quoted 11 lines · Stephen Boyd
Quoted 6 lines · Rajendra Nayak

msm8974 has gcc and mmcc nodes, and apq8084 has a gcc node which implement gdsc powerdomains. Add the #power-domain-cells property to them.

Signed-off-by Rajendra Nayak <[email protected]>

---

This needs to go through Andy's tree and arm-soc. Also, I don't see an update for the binding anywhere. Please update the binding too.

Will do, I hope you mean update the bindings in qcom,mmcc.txt and qcom,gcc.txt?

#22 lore raw

PATCH v8 10/13 arm: dts: qcom: Add #power-domain-cells property

Quoted 9 lines · Rajendra Nayak
Quoted 8 lines · Stephen Boyd
Quoted 6 lines · Rajendra Nayak

msm8974 has gcc and mmcc nodes, and apq8084 has a gcc node which implement gdsc powerdomains. Add the #power-domain-cells property to them.

Signed-off-by Rajendra Nayak <[email protected]>

---

Quoted 6 lines
Quoted 3 lines

This needs to go through Andy's tree and arm-soc. Also, I don't see an update for the binding anywhere. Please update the binding too.

Will do, I hope you mean update the bindings in qcom,mmcc.txt and qcom,gcc.txt?

Yes.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#23 lore raw

PATCH v8 11/13 clk: qcom: gdsc: Use PM clocks to control gdsc clocks

In reply to Rajendra Nayak

The devices within a gdsc power domain, quite often have additional clocks to be turned on/off along with the power domain itself. Once the drivers for these devices are converted to use runtime PM, it would be possible to remove all clock handling from the drivers if the gdsc driver can handle it.
Use PM clocks to add support for this. A list of clock ids specified per gdsc would be the clocks turned on/off on every device start/stop callbacks.

Signed-off-by Rajendra Nayak <[email protected]>
---
 drivers/clk/qcom/gdsc.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/gdsc.h |  7 ++++++
 2 files changed, 65 insertions(+)
drivers/clk/qcom/gdsc.c +58 -0
index da9fad8..ec1dfb5 100644
@@ -12,10 +12,12 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
+#include <linux/pm_clock.h>
 #include <linux/pm_domain.h>
 #include <linux/regmap.h>
 #include <linux/reset-controller.h>

@@ -161,6 +163,59 @@ static int gdsc_disable(struct generic_pm_domain *domain)
 	return gdsc_toggle_logic(sc, false);
 }
 
+static inline bool match(unsigned int id, unsigned int *ids, unsigned int count)
+{
+	int i;
+
+	for (i = 0; i < count; i++)
+		if (id == ids[i])
+			return true;
+	return false;
+}
+
+static int gdsc_attach(struct generic_pm_domain *domain, struct device *dev)
+{
+	int ret, i = 0, j = 0;
+	struct gdsc *sc = domain_to_gdsc(domain);
+	struct of_phandle_args clkspec;
+	struct device_node *np = dev->of_node;
+
+	if (!sc->clock_count)
+		return 0;
+
+	ret = pm_clk_create(dev);
+	if (ret) {
+		dev_dbg(dev, "pm_clk_create failed %d\n", ret);
+		return ret;
+	}
+
+	sc->clks = devm_kcalloc(dev, sc->clock_count, sizeof(sc->clks),
+				       GFP_KERNEL);
+	if (!sc->clks)
+		return -ENOMEM;
+
+	while (!of_parse_phandle_with_args(np, "clocks", "#clock-cells", i,
+					   &clkspec)) {
+		if (match(clkspec.args[0], sc->clocks, sc->clock_count)) {
+			sc->clks[j] = of_clk_get_from_provider(&clkspec);
+			pm_clk_add_clk(dev, sc->clks[j]);
+			j++;
+		}
+		i++;
+	}
+	return 0;
+};
+
+static void gdsc_detach(struct generic_pm_domain *domain, struct device *dev)
+{
+	struct gdsc *sc = domain_to_gdsc(domain);
+
+	if (!sc->clock_count)
+		return;
+
+	pm_clk_destroy(dev);
+};
+
 static int gdsc_init(struct gdsc *sc)
 {
 	u32 mask, val;

@@ -196,6 +251,9 @@ static int gdsc_init(struct gdsc *sc)
 
 	sc->pd.power_off = gdsc_disable;
 	sc->pd.power_on = gdsc_enable;
+	sc->pd.attach_dev = gdsc_attach;
+	sc->pd.detach_dev = gdsc_detach;
+	sc->pd.flags = GENPD_FLAG_PM_CLK;
 	pm_genpd_init(&sc->pd, NULL, !on);
 
 	return 0;
drivers/clk/qcom/gdsc.h +7 -0
index 5ded268..2fdb332 100644
@@ -17,6 +17,7 @@
 #include <linux/err.h>
 #include <linux/pm_domain.h>
 
+struct clk;
 struct regmap;
 struct reset_controller_dev;
 

@@ -38,6 +39,9 @@ struct reset_controller_dev;
  * @resets: ids of resets associated with this gdsc
  * @reset_count: number of @resets
  * @rcdev: reset controller
+ * @clocks: ids of clocks associated with the gdsc
+ * @clock_count: number of @clocks
+ * @clks: clock pointers to gdsc clocks
  */
 struct gdsc {
 	struct generic_pm_domain	pd;

@@ -49,6 +53,9 @@ struct gdsc {
 	struct reset_controller_dev	*rcdev;
 	unsigned int			*resets;
 	unsigned int			reset_count;
+	unsigned int			*clocks;
+	unsigned int			clock_count;
+	struct clk			**clks;
 };
 
 #ifdef CONFIG_QCOM_GDSC
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#24 lore raw

PATCH v8 11/13 clk: qcom: gdsc: Use PM clocks to control gdsc clocks

Quoted 15 lines · Rajendra Nayak
drivers/clk/qcom/gdsc.c +15 -0
+
+static int gdsc_attach(struct generic_pm_domain *domain, struct device *dev)
+{
+	int ret, i = 0, j = 0;
+	struct gdsc *sc = domain_to_gdsc(domain);
+	struct of_phandle_args clkspec;
+	struct device_node *np = dev->of_node;
+
+	if (!sc->clock_count)
+		return 0;
+
+	ret = pm_clk_create(dev);
+	if (ret) {
+		dev_dbg(dev, "pm_clk_create failed %d\n", ret);
+		return ret;

drivers/clk/qcom/gdsc.c static int gdsc_disable(struct generic_pm_domain *domain) · line 199

Quoted 10 lines
drivers/clk/qcom/gdsc.c +10 -0
+	}
+
+	sc->clks = devm_kcalloc(dev, sc->clock_count, sizeof(sc->clks),
+				       GFP_KERNEL);
+	if (!sc->clks)
+		return -ENOMEM;
+
+	while (!of_parse_phandle_with_args(np, "clocks", "#clock-cells", i,
+					   &clkspec)) {
+		if (match(clkspec.args[0], sc->clocks, sc->clock_count)) {

I'm lost. I was hoping we could just make up a clkspec on the stack and pass it over to of_clk_get_from_provider() without having to go through the np of the client device. The point being to avoid forcing this code from knowing about the consumer binding or connection name choice for each device. Instead, it assumes that it's a #clock-cells=<1> binding and gets the clocks by passing the 1 cell data without calling of_parse_phandle_with_args().

Now, one downside of that approach is that it's DT centric (also of_clk_get_from_provider() is not an exported symbol yet). So I'm really starting to lean towards exposing __clk_create_clk() (or some better named "provider" function) that will allow clk providers to turn their clk_hw structure into a struct clk pointer. That avoids the DT centric design, and avoids binding the provider to the connection ids too.

Quoted 8 lines
drivers/clk/qcom/gdsc.c +8 -0
+			sc->clks[j] = of_clk_get_from_provider(&clkspec);
+			pm_clk_add_clk(dev, sc->clks[j]);
+			j++;
+		}
+		i++;
+	}
+	return 0;
+};
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#25 lore raw

PATCH v8 11/13 clk: qcom: gdsc: Use PM clocks to control gdsc clocks

Quoted 22 lines · Stephen Boyd
Quoted 21 lines · Rajendra Nayak
drivers/clk/qcom/gdsc.c +21 -0
+
+static int gdsc_attach(struct generic_pm_domain *domain, struct device *dev)
+{
+	int ret, i = 0, j = 0;
+	struct gdsc *sc = domain_to_gdsc(domain);
+	struct of_phandle_args clkspec;
+	struct device_node *np = dev->of_node;
+
+	if (!sc->clock_count)
+		return 0;
+
+	ret = pm_clk_create(dev);
+	if (ret) {
+		dev_dbg(dev, "pm_clk_create failed %d\n", ret);
+		return ret;
+	}
+
+	sc->clks = devm_kcalloc(dev, sc->clock_count, sizeof(sc->clks),
+				       GFP_KERNEL);
+	if (!sc->clks)
+		return -ENOMEM;
Quoted 8 lines

drivers/clk/qcom/gdsc.c static int gdsc_disable(struct generic_pm_domain *domain) · line 199

Quoted 4 lines
drivers/clk/qcom/gdsc.c +4 -0
+
+	while (!of_parse_phandle_with_args(np, "clocks", "#clock-cells", i,
+					   &clkspec)) {
+		if (match(clkspec.args[0], sc->clocks, sc->clock_count)) {

I'm lost. I was hoping we could just make up a clkspec on the stack and pass it over to of_clk_get_from_provider() without having to go through the np of the client device.

Sorry, can you elaborate a little more on 'make up a clkspec on the stack'? I don't seem to be able to figure out.

The point being

Quoted 2 lines

to avoid forcing this code from knowing about the consumer binding or connection name choice for each device. Instead, it

I am not associating connection names but internal clock ids with the corresponding gdsc. I don;t see how this is forcing the code to know consumer DT bindings. However, it is forcing the client device to have all clocks that need to be controlled listed in DT, which I assume should be the case.

Quoted 3 lines

assumes that it's a #clock-cells=<1> binding and gets the clocks by passing the 1 cell data without calling of_parse_phandle_with_args().

Yes, the code did assume #clock-cells=<1>, which is what all of the existing clock controllers used in QCOM SoCs use.

Quoted 8 lines

Now, one downside of that approach is that it's DT centric (also of_clk_get_from_provider() is not an exported symbol yet). So I'm really starting to lean towards exposing __clk_create_clk() (or some better named "provider" function) that will allow clk providers to turn their clk_hw structure into a struct clk pointer. That avoids the DT centric design, and avoids binding the provider to the connection ids too.

Again, there are no connection ids used here. I can explore the approach of associating clk_hw structs to gdscs and avoiding anything to do with a DT lookup. I seem to however fail to understand why we need this to have nothing to do with DT though. Do you think the clocks for a given device which need to be controlled along with the power switch (gdsc) should *not* be associated/listed within its DT node?

author
#26 lore raw

PATCH v8 11/13 clk: qcom: gdsc: Use PM clocks to control gdsc clocks

In reply to Stephen Boyd

Hi Stephen,

Quoted 35 lines
Quoted 15 lines

+static int gdsc_attach(struct generic_pm_domain *domain, struct device *dev)

drivers/clk/qcom/gdsc.c +13 -0
+{
+	int ret, i = 0, j = 0;
+	struct gdsc *sc = domain_to_gdsc(domain);
+	struct of_phandle_args clkspec;
+	struct device_node *np = dev->of_node;
+
+	if (!sc->clock_count)
+		return 0;
+
+	ret = pm_clk_create(dev);
+	if (ret) {
+		dev_dbg(dev, "pm_clk_create failed %d\n", ret);
+		return ret;

drivers/clk/qcom/gdsc.c static int gdsc_disable(struct generic_pm_domain *domain) · line 199

Quoted 10 lines
drivers/clk/qcom/gdsc.c +10 -0
+	}
+
+	sc->clks = devm_kcalloc(dev, sc->clock_count, sizeof(sc->clks),
+				       GFP_KERNEL);
+	if (!sc->clks)
+		return -ENOMEM;
+
+	while (!of_parse_phandle_with_args(np, "clocks", "#clock-cells", i,
+					   &clkspec)) {
+		if (match(clkspec.args[0], sc->clocks, sc->clock_count)) {

I'm lost. I was hoping we could just make up a clkspec on the stack and pass it over to of_clk_get_from_provider() without having to go through the np of the client device. The point being to avoid forcing this code from knowing about the consumer binding or connection name choice for each device. Instead, it assumes that it's a #clock-cells=<1> binding and gets the clocks by passing the 1 cell data without calling of_parse_phandle_with_args().

Quoted 7 lines

Now, one downside of that approach is that it's DT centric (also of_clk_get_from_provider() is not an exported symbol yet). So I'm really starting to lean towards exposing __clk_create_clk() (or some better named "provider" function) that will allow clk providers to turn their clk_hw structure into a struct clk pointer. That avoids the DT centric design, and avoids binding the provider to the connection ids too.

Stephen, I started to relook at these patches, avoiding the DT centric design and implementing a clk helper API as you suggested above.

While this would work for GDSCs with just one device, its hard to scale if we ever run into GDSCs with multiple devices (In which case you need to know which device within the GDSC needs which clocks)

Do you think its a fair limitation (one device per gdsc) to live with? because I can't seem to figure how a non DT centric design would otherwise work.

regards,
Rajendra

Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#27 lore raw

PATCH v8 11/13 clk: qcom: gdsc: Use PM clocks to control gdsc clocks

Quoted 18 lines · Rajendra Nayak
Quoted 10 lines

I'm lost. I was hoping we could just make up a clkspec on the stack and pass it over to of_clk_get_from_provider() without having to go through the np of the client device. The point being to avoid forcing this code from knowing about the consumer binding or connection name choice for each device. Instead, it assumes that it's a #clock-cells=<1> binding and gets the clocks by passing the 1 cell data without calling of_parse_phandle_with_args().

Quoted 7 lines

Now, one downside of that approach is that it's DT centric (also of_clk_get_from_provider() is not an exported symbol yet). So I'm really starting to lean towards exposing __clk_create_clk() (or some better named "provider" function) that will allow clk providers to turn their clk_hw structure into a struct clk pointer. That avoids the DT centric design, and avoids binding the provider to the connection ids too.

Quoted 6 lines

Stephen, I started to relook at these patches, avoiding the DT centric design and implementing a clk helper API as you suggested above.

While this would work for GDSCs with just one device, its hard to scale if we ever run into GDSCs with multiple devices (In which case you need to know which device within the GDSC needs which clocks)

Why? The GDSC should know which clocks it's forcing on and off and it shouldn't need to care which devices are consuming those clocks. The GDSC structure would have a bunch of clk_hw pointers that it could convert into struct clk pointer to do clk consumer API things as needed.

Maybe I'm missing something? If we're trying to take away clock control from our device drivers and hide that in SoC glue code (something we almost have to do so that the ordering of clocks vs. GDSC enable isn't wrong), then that isn't a GDSC, but some linux genpd concept. I imagine it would be a genpd per hw block (or device driver really) and then those genpds would be children of a larger GDSC genpd. For example, two hw blocks could have separate genpds for their clock control so that we can hide the clock on/off from the drivers, and then those are children of a single GDSC backed genpd for the physical domain they both reside in. If the GDSC domain and the sw domain need to control the same clocks, everything will work because we reference count appropriately.

The downside of this all is that we're putting a bunch of knowledge about which drivers are using which clocks into the SoC clock driver. But I don't know where else we would put this if we want to hide these details from the driver authors. The only other option is to do it with DT, and the whole DT ABI there scares me enough to want to try and make it work in the SoC clock driver for now.

Quoted 4 lines

Do you think its a fair limitation (one device per gdsc) to live with? because I can't seem to figure how a non DT centric design would otherwise work.

No. The SMMU use case on a-family has SMMU and the hw block (video, gpu, display) that uses it within the same GDSC power domain.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#28 lore raw

PATCH v8 12/13 clk: qcom: gdsc: Enable an RCG before turing on the gdsc

In reply to Rajendra Nayak

Some gdsc instances require a certain root clock (RCG) to be turned on *before* the power domain itself can be turned on. Handle this as part of the gdsc enable/disable callbacks.

Signed-off-by Rajendra Nayak <[email protected]>
---
 drivers/clk/qcom/gcc-msm8916.c |  1 +
 drivers/clk/qcom/gdsc.c        | 14 ++++++++++++--
 drivers/clk/qcom/gdsc.h        |  4 ++++
 3 files changed, 17 insertions(+), 2 deletions(-)
drivers/clk/qcom/gcc-msm8916.c +1 -0
index 10ba745..bb34345 100644
@@ -2601,6 +2601,7 @@ static struct gdsc oxili_gdsc = {
 		.name = "oxili",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
+	.root_clock = GFX3D_CLK_SRC,
 };
 
 static struct clk_regmap *gcc_msm8916_clocks[] = {
drivers/clk/qcom/gdsc.c +12 -2
index ec1dfb5..20965fc 100644
@@ -131,6 +131,9 @@ static int gdsc_enable(struct generic_pm_domain *domain)
 	if (sc->pwrsts == PWRSTS_ON)
 		return gdsc_deassert_reset(sc);
 
+	if (sc->root_clk)
+		clk_prepare_enable(sc->root_clk);
+
 	ret = gdsc_toggle_logic(sc, true);
 	if (ret)
 		return ret;

@@ -152,6 +155,7 @@ static int gdsc_enable(struct generic_pm_domain *domain)
 
 static int gdsc_disable(struct generic_pm_domain *domain)
 {
+	int ret;
 	struct gdsc *sc = domain_to_gdsc(domain);
 
 	if (sc->pwrsts == PWRSTS_ON)

@@ -160,7 +164,12 @@ static int gdsc_disable(struct generic_pm_domain *domain)
 	if (sc->pwrsts & PWRSTS_OFF)
 		gdsc_clear_mem_on(sc);
 
-	return gdsc_toggle_logic(sc, false);
+	ret = gdsc_toggle_logic(sc, false);
+
+	if (sc->root_clk)
+		clk_disable_unprepare(sc->root_clk);
+
+	return ret;
 }
 
 static inline bool match(unsigned int id, unsigned int *ids, unsigned int count)

@@ -200,7 +209,8 @@ static int gdsc_attach(struct generic_pm_domain *domain, struct device *dev)
 			sc->clks[j] = of_clk_get_from_provider(&clkspec);
 			pm_clk_add_clk(dev, sc->clks[j]);
 			j++;
-		}
+		} else if (clkspec.args[0] == sc->root_clock)
+			sc->root_clk = of_clk_get_from_provider(&clkspec);
 		i++;
 	}
 	return 0;
drivers/clk/qcom/gdsc.h +4 -0
index 2fdb332..0b958a6 100644
@@ -42,6 +42,8 @@ struct reset_controller_dev;
  * @clocks: ids of clocks associated with the gdsc
  * @clock_count: number of @clocks
  * @clks: clock pointers to gdsc clocks
+ * @root_clock: id of the root clock to be enabled
+ * @root_clk: root clk pointer
  */
 struct gdsc {
 	struct generic_pm_domain	pd;

@@ -56,6 +58,8 @@ struct gdsc {
 	unsigned int			*clocks;
 	unsigned int			clock_count;
 	struct clk			**clks;
+	unsigned int			root_clock;
+	struct clk			*root_clk;
 };
 
 #ifdef CONFIG_QCOM_GDSC
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
author
#29 lore raw

PATCH v8 13/13 clk: qcom: gdsc: Manage clocks with !CONFIG_PM

In reply to Rajendra Nayak

With CONFIG_PM disabled, turn the devices clocks on during driver binding to the device, and turn them off when the driver is unbound from the device. Platforms can specify all the clocks that need to be managed in !CONFIG_PM case using qcom_pm_add_notifier().

The use of pm_clk_add_notifier() isn't appropriate here since we need to only manage clocks with valid power domain associations done via DT, instead of what pm_clk_add_notifier() does, which is manage clocks for all on SoC/off SoC devices associating all of them to a dummy power domain instead

Signed-off-by Rajendra Nayak <[email protected]>
---
 drivers/clk/qcom/gdsc.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/gdsc.h |  8 ++++++
 2 files changed, 82 insertions(+)
drivers/clk/qcom/gdsc.c +74 -0
index 20965fc..afc0b15 100644
@@ -17,6 +17,7 @@
 #include <linux/err.h>
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
+#include <linux/platform_device.h>
 #include <linux/pm_clock.h>
 #include <linux/pm_domain.h>
 #include <linux/regmap.h>

@@ -303,3 +304,76 @@ void gdsc_unregister(struct device *dev)
 {
 	of_genpd_del_provider(dev->of_node);
 }
+
+#ifndef CONFIG_PM
+static void enable_clock(struct of_phandle_args *clkspec)
+{
+	struct clk *clk;
+
+	clk = of_clk_get_from_provider(clkspec);
+	if (!IS_ERR(clk)) {
+		clk_prepare_enable(clk);
+		clk_put(clk);
+	}
+}
+
+static void disable_clock(struct of_phandle_args *clkspec)
+{
+	struct clk *clk;
+
+	clk = of_clk_get_from_provider(clkspec);
+	if (!IS_ERR(clk)) {
+		clk_disable_unprepare(clk);
+		clk_put(clk);
+	}
+}
+
+static int clk_notify(struct notifier_block *nb, unsigned long action,
+		      void *data)
+{
+	int sz, i = 0;
+	struct device *dev = data;
+	struct gdsc_notifier_block *gdsc_nb;
+	struct of_phandle_args clkspec;
+	struct device_node *np = dev->of_node;
+
+	if (!of_find_property(dev->of_node, "power-domains", &sz))
+		return 0;
+
+	gdsc_nb = container_of(nb, struct gdsc_notifier_block, nb);
+
+	if (!gdsc_nb->clock_count)
+		return 0;
+
+	switch (action) {
+	case BUS_NOTIFY_BIND_DRIVER:
+		while (!of_parse_phandle_with_args(np, "clocks", "#clock-cells",
+						   i, &clkspec)) {
+			if (match(clkspec.args[0], gdsc_nb->clocks,
+				  gdsc_nb->clock_count))
+				enable_clock(&clkspec);
+			i++;
+		}
+		break;
+	case BUS_NOTIFY_UNBOUND_DRIVER:
+		while (!of_parse_phandle_with_args(np, "clocks", "#clock-cells",
+						   i, &clkspec)) {
+			if (match(clkspec.args[0], gdsc_nb->clocks,
+				  gdsc_nb->clock_count))
+				disable_clock(&clkspec);
+			i++;
+		}
+		break;
+	}
+	return 0;
+}
+
+void qcom_pm_add_notifier(struct gdsc_notifier_block *gdsc_nb)
+{
+	if (!gdsc_nb)
+		return;
+
+	gdsc_nb->nb.notifier_call = clk_notify,
+	bus_register_notifier(&platform_bus_type, &gdsc_nb->nb);
+}
+#endif
drivers/clk/qcom/gdsc.h +8 -0
index 0b958a6..b0a36ed 100644
@@ -76,4 +76,12 @@ static inline int gdsc_register(struct device *d, struct gdsc **g, size_t n,
 
 static inline void gdsc_unregister(struct device *d) {};
 #endif /* CONFIG_QCOM_GDSC */
+#ifndef CONFIG_PM
+struct gdsc_notifier_block {
+	struct notifier_block nb;
+	unsigned int	*clocks;
+	unsigned int	clock_count;
+};
+void qcom_pm_add_notifier(struct gdsc_notifier_block *);
+#endif /* !CONFIG_PM */
 #endif /* __QCOM_GDSC_H__ */
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation