[PATCH 00/16] Modernize rest of the krait drivers

[PATCH 00/16] Modernize rest of the krait drivers

[Ansuel Smith]

This is a follow-up to the ipq806x gcc modernize series. Manu cleanup
changes and also some discoveries of wrong definition notice only with
all these conversions.

The first patch is an improvement of the clk_hw_get_parent_index. The
original idea of clk_hw_get_parent_index was to give a way to access the
parent index but for some reason the final version limited it to the
current index. We change it to give the current parent if is not
provided and to give the requested parent if provided. Any user of this
function is updated to follow the new implementation.

The patch 2 and 3 are some additional fixes for gcc.
The first one is a fix that register the pxo and cxo fixed clock only if
they are not defined in DTS.
The patch 3 require some explaination. In short is a big HACK to prevent
kernel panic with this series.

The kpss-xcc driver is a mess.
The Documentation declare that the clocks should be provided but for some
reason it was never followed.
In fact in the ipq8064 DTSI only the clocks for l2cc are declared but
for cpu0 and cpu1 the clocks are not defined.
The kpss-xcc driver use parent_names so the clks are ignored and never
used so till now it wasn't a problem (ignoring the fact that they
doesn't follow documentation at all)
On top of that, the l2cc node declare the pxo clock in a really strange
way. It's declared using the PXO_SRC gcc clock that is never defined in
the gcc ipq8064 clock table. (the correct way was to declare a fixed
clock in dts and reference that)
To prevent any kind of problem we use the patch 3 and provide the clk
for PXO_SRC in the gcc clock table. We manually provide the clk after
gcc probe.

Patch 4 is just a minor cleanup where we use the poll macro

Patch 5 is the actually kpss-xcc conversion to parent data

Patch 6-7 should be a fixup of a real conver case

Patch 8 converts the krait-cc to parent_data
Patch 9 give some love to the code with some minor fixup
Patch 10 drop the hardcoded safe sel and use the new
clk_hw_get_parent_index to get the safe parent index.
(also I discovered that the parent order was wrong)

Patch 11 is an additional fixup to force the reset of the muxes even
more.

Patch 12-13 are some additiona taken from the qsdk that were missing in
the upstream driver

Patch 14 converts krait-cc to yaml (should i also convert the kpss-scc
driver?)

Patch 15 finally adds all this stuff to the ipq8064 dtsi (and fix the
stupid PXO_SRC phandle)

Patch 16 conver the kpss driver to yaml and fix some Docuemntation errors

I tested this series on a ipq8064 SoC by running a cache benchmark test
to make sure the changes are correct and we don't silently cause
regressions. Also I compared the output of the clk_summary every time
and we finally have a sane output where the mux are correctly placed in
the correct parent. (till now we had the cpu aux clock all over the
place, probably never cause problems but who knows.)

Ansuel Smith (16):
  clk: permit to define a custom parent for clk_hw_get_parent_index
  clk: qcom: gcc-ipq806x: skip pxo/cxo fixed clk if already present
  clk: qcom: gcc-ipq806x: add PXO_SRC in clk table
  clk: qcom: clk-hfpll: use poll_timeout macro
  clk: qcom: kpss-xcc: convert to parent data API
  clk: qcom: clk-krait: unlock spin after mux completion
  clk: qcom: clk-krait: add hw_parent check for div2_round_rate
  clk: qcom: krait-cc: convert to parent_data API
  clk: qcom: krait-cc: drop pr_info and register qsb only if needed
  clk: qcom: krait-cc: drop hardcoded safe_sel
  clk: qcom: krait-cc: force sec_mux to QSB
  clk: qcom: clk-krait: add 8064 errata workaround
  clk: qcom: clk-krait: add enable disable ops
  dt-bindings: clock: Convert qcom,krait-cc to yaml
  dts: qcom-ipq8064: add missing krait-cc compatible and clocks
  dt-bindings: arm: msm: Convert kpss driver Documentation to yaml

 .../bindings/arm/msm/qcom,kpss-acc.txt        |  49 -----
 .../bindings/arm/msm/qcom,kpss-acc.yaml       |  97 +++++++++
 .../bindings/arm/msm/qcom,kpss-gcc.txt        |  44 ----
 .../bindings/arm/msm/qcom,kpss-gcc.yaml       |  62 ++++++
 .../bindings/clock/qcom,krait-cc.txt          |  34 ---
 .../bindings/clock/qcom,krait-cc.yaml         |  63 ++++++
 arch/arm/boot/dts/qcom-ipq8064.dtsi           |  20 +-
 drivers/clk/clk.c                             |  14 +-
 drivers/clk/qcom/clk-hfpll.c                  |  13 +-
 drivers/clk/qcom/clk-krait.c                  |  44 +++-
 drivers/clk/qcom/clk-krait.h                  |   1 +
 drivers/clk/qcom/gcc-ipq806x.c                |  27 ++-
 drivers/clk/qcom/kpss-xcc.c                   |  25 +--
 drivers/clk/qcom/krait-cc.c                   | 201 ++++++++++--------
 drivers/clk/tegra/clk-periph.c                |   2 +-
 drivers/clk/tegra/clk-sdmmc-mux.c             |   2 +-
 drivers/clk/tegra/clk-super.c                 |   4 +-
 include/linux/clk-provider.h                  |   2 +-
 18 files changed, 448 insertions(+), 256 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
 create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.yaml
 delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
 create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/qcom,krait-cc.txt
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,krait-cc.yaml

-- 
2.34.1

[PATCH 01/16] clk: permit to define a custom parent for clk_hw_get_parent_index

[Ansuel Smith]

Clk can have multiple parents. Some clk may require to get the cached
index of other parent that are not current associated with the clk.
Extend clk_hw_get_parent_index() with an optional parent to permit a
driver to get the cached index. If parent is NULL, the parent associated
with the provided hw clk is used.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/clk.c                 | 14 +++++++++-----
 drivers/clk/tegra/clk-periph.c    |  2 +-
 drivers/clk/tegra/clk-sdmmc-mux.c |  2 +-
 drivers/clk/tegra/clk-super.c     |  4 ++--
 include/linux/clk-provider.h      |  2 +-
 5 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8de6a22498e7..fe42f56bfbdf 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1711,15 +1711,19 @@ static int clk_fetch_parent_index(struct clk_core *core,
 /**
  * clk_hw_get_parent_index - return the index of the parent clock
  * @hw: clk_hw associated with the clk being consumed
+ * @parent: optional clk_hw of the parent to be fetched
  *
- * Fetches and returns the index of parent clock. Returns -EINVAL if the given
- * clock does not have a current parent.
+ * Fetches and returns the index of parent clock. If parent is not
+ * provided the parent of hw is used.
+ * Returns -EINVAL if the given clock does not have a current parent.
  */
-int clk_hw_get_parent_index(struct clk_hw *hw)
+int clk_hw_get_parent_index(struct clk_hw *hw, struct clk_hw *parent)
 {
-	struct clk_hw *parent = clk_hw_get_parent(hw);
+	/* With parent NULL get the current parent of hw */
+	if (!parent)
+		parent = clk_hw_get_parent(hw);
 
-	if (WARN_ON(parent == NULL))
+	if (WARN_ON(!parent))
 		return -EINVAL;
 
 	return clk_fetch_parent_index(hw->core, parent->core);
diff --git a/drivers/clk/tegra/clk-periph.c b/drivers/clk/tegra/clk-periph.c
index 79ca3aa072b7..0fecdbbaca8f 100644
--- a/drivers/clk/tegra/clk-periph.c
+++ b/drivers/clk/tegra/clk-periph.c
@@ -116,7 +116,7 @@ static void clk_periph_restore_context(struct clk_hw *hw)
 	struct clk_hw *div_hw = &periph->divider.hw;
 	int parent_id;
 
-	parent_id = clk_hw_get_parent_index(hw);
+	parent_id = clk_hw_get_parent_index(hw, NULL);
 	if (WARN_ON(parent_id < 0))
 		return;
 
diff --git a/drivers/clk/tegra/clk-sdmmc-mux.c b/drivers/clk/tegra/clk-sdmmc-mux.c
index 4f2c3309eea4..6013ca8444f4 100644
--- a/drivers/clk/tegra/clk-sdmmc-mux.c
+++ b/drivers/clk/tegra/clk-sdmmc-mux.c
@@ -210,7 +210,7 @@ static void clk_sdmmc_mux_restore_context(struct clk_hw *hw)
 	unsigned long rate = clk_hw_get_rate(hw);
 	int parent_id;
 
-	parent_id = clk_hw_get_parent_index(hw);
+	parent_id = clk_hw_get_parent_index(hw, NULL);
 	if (WARN_ON(parent_id < 0))
 		return;
 
diff --git a/drivers/clk/tegra/clk-super.c b/drivers/clk/tegra/clk-super.c
index a98a420398fa..27cbbc09ef68 100644
--- a/drivers/clk/tegra/clk-super.c
+++ b/drivers/clk/tegra/clk-super.c
@@ -128,7 +128,7 @@ static void clk_super_mux_restore_context(struct clk_hw *hw)
 {
 	int parent_id;
 
-	parent_id = clk_hw_get_parent_index(hw);
+	parent_id = clk_hw_get_parent_index(hw, NULL);
 	if (WARN_ON(parent_id < 0))
 		return;
 
@@ -180,7 +180,7 @@ static void clk_super_restore_context(struct clk_hw *hw)
 	struct clk_hw *div_hw = &super->frac_div.hw;
 	int parent_id;
 
-	parent_id = clk_hw_get_parent_index(hw);
+	parent_id = clk_hw_get_parent_index(hw, NULL);
 	if (WARN_ON(parent_id < 0))
 		return;
 
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 2faa6f7aa8a8..65b2850c09be 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -1198,7 +1198,7 @@ unsigned int clk_hw_get_num_parents(const struct clk_hw *hw);
 struct clk_hw *clk_hw_get_parent(const struct clk_hw *hw);
 struct clk_hw *clk_hw_get_parent_by_index(const struct clk_hw *hw,
 					  unsigned int index);
-int clk_hw_get_parent_index(struct clk_hw *hw);
+int clk_hw_get_parent_index(struct clk_hw *hw, struct clk_hw *parent);
 int clk_hw_set_parent(struct clk_hw *hw, struct clk_hw *new_parent);
 unsigned int __clk_get_enable_count(struct clk *clk);
 unsigned long clk_hw_get_rate(const struct clk_hw *hw);
-- 
2.34.1

[PATCH 02/16] clk: qcom: gcc-ipq806x: skip pxo/cxo fixed clk if already present

[Ansuel Smith]

Skip pxo/cxo clock registration if they are already defined in DTS as fixed
clock.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/gcc-ipq806x.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
index d6b7adb4be38..27f6d7626abb 100644
--- a/drivers/clk/qcom/gcc-ipq806x.c
+++ b/drivers/clk/qcom/gcc-ipq806x.c
@@ -10,6 +10,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/regmap.h>
 #include <linux/reset-controller.h>
@@ -3061,15 +3062,22 @@ static int gcc_ipq806x_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct regmap *regmap;
+	struct clk *clk;
 	int ret;
 
-	ret = qcom_cc_register_board_clk(dev, "cxo_board", "cxo", 25000000);
-	if (ret)
-		return ret;
+	clk = clk_get(dev, "cxo");
+	if (IS_ERR(clk)) {
+		ret = qcom_cc_register_board_clk(dev, "cxo_board", "cxo", 25000000);
+		if (ret)
+			return ret;
+	}
 
-	ret = qcom_cc_register_board_clk(dev, "pxo_board", "pxo", 25000000);
-	if (ret)
-		return ret;
+	clk = clk_get(dev, "pxo");
+	if (IS_ERR(clk)) {
+		ret = qcom_cc_register_board_clk(dev, "pxo_board", "pxo", 25000000);
+		if (ret)
+			return ret;
+	}
 
 	ret = qcom_cc_probe(pdev, &gcc_ipq806x_desc);
 	if (ret)
-- 
2.34.1

[PATCH 03/16] clk: qcom: gcc-ipq806x: add PXO_SRC in clk table

[Ansuel Smith]

PXO_SRC is currently defined in the gcc include and referenced in the
ipq8064 DTSI. Correctly provide a clk after gcc probe to fix kernel
panic if a driver starts to actually use it.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/gcc-ipq806x.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
index 27f6d7626abb..7271d3afdc89 100644
--- a/drivers/clk/qcom/gcc-ipq806x.c
+++ b/drivers/clk/qcom/gcc-ipq806x.c
@@ -26,6 +26,8 @@
 #include "clk-hfpll.h"
 #include "reset.h"
 
+static struct clk_regmap pxo = { };
+
 static struct clk_pll pll0 = {
 	.l_reg = 0x30c4,
 	.m_reg = 0x30c8,
@@ -2754,6 +2756,7 @@ static struct clk_dyn_rcg ubi32_core2_src_clk = {
 };
 
 static struct clk_regmap *gcc_ipq806x_clks[] = {
+	[PXO_SRC] = NULL,
 	[PLL0] = &pll0.clkr,
 	[PLL0_VOTE] = &pll0_vote,
 	[PLL3] = &pll3.clkr,
@@ -3083,6 +3086,10 @@ static int gcc_ipq806x_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	clk = clk_get(dev, "pxo");
+	pxo.hw = *__clk_get_hw(clk);
+	gcc_ipq806x_clks[PXO_SRC] = &pxo;
+
 	regmap = dev_get_regmap(dev, NULL);
 	if (!regmap)
 		return -ENODEV;
-- 
2.34.1

[PATCH 04/16] clk: qcom: clk-hfpll: use poll_timeout macro

[Ansuel Smith]

Use regmap_read_poll_timeout macro instead of do-while structure.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/clk-hfpll.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/qcom/clk-hfpll.c b/drivers/clk/qcom/clk-hfpll.c
index e847d586a73a..a4e347eb4d4d 100644
--- a/drivers/clk/qcom/clk-hfpll.c
+++ b/drivers/clk/qcom/clk-hfpll.c
@@ -12,6 +12,8 @@
 #include "clk-regmap.h"
 #include "clk-hfpll.h"
 
+#define HFPLL_BUSY_WAIT_TIMEOUT	100
+
 #define PLL_OUTCTRL	BIT(0)
 #define PLL_BYPASSNL	BIT(1)
 #define PLL_RESET_N	BIT(2)
@@ -72,13 +74,12 @@ static void __clk_hfpll_enable(struct clk_hw *hw)
 	regmap_update_bits(regmap, hd->mode_reg, PLL_RESET_N, PLL_RESET_N);
 
 	/* Wait for PLL to lock. */
-	if (hd->status_reg) {
-		do {
-			regmap_read(regmap, hd->status_reg, &val);
-		} while (!(val & BIT(hd->lock_bit)));
-	} else {
+	if (hd->status_reg)
+		regmap_read_poll_timeout(regmap, hd->status_reg, val,
+					 !(val & BIT(hd->lock_bit)), USEC_PER_MSEC * 2,
+					 HFPLL_BUSY_WAIT_TIMEOUT * USEC_PER_MSEC);
+	else
 		udelay(60);
-	}
 
 	/* Enable PLL output. */
 	regmap_update_bits(regmap, hd->mode_reg, PLL_OUTCTRL, PLL_OUTCTRL);
-- 
2.34.1

[PATCH 05/16] clk: qcom: kpss-xcc: convert to parent data API

[Ansuel Smith]

Convert the driver to parent data API. From the Documentation pll8_vote
and pxo should be declared in the DTS so fw_name can be used instead of
parent_names. Name is still used to save regression on old definition.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/kpss-xcc.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/qcom/kpss-xcc.c b/drivers/clk/qcom/kpss-xcc.c
index 4fec1f9142b8..347f70e9f5fe 100644
--- a/drivers/clk/qcom/kpss-xcc.c
+++ b/drivers/clk/qcom/kpss-xcc.c
@@ -12,9 +12,9 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 
-static const char *aux_parents[] = {
-	"pll8_vote",
-	"pxo",
+static const struct clk_parent_data aux_parents[] = {
+	{ .name = "pll8_vote", .fw_name = "pll8_vote" },
+	{ .name = "pxo", .fw_name = "pxo" },
 };
 
 static unsigned int aux_parent_map[] = {
@@ -32,8 +32,8 @@ MODULE_DEVICE_TABLE(of, kpss_xcc_match_table);
 static int kpss_xcc_driver_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *id;
-	struct clk *clk;
 	void __iomem *base;
+	struct clk_hw *hw;
 	const char *name;
 
 	id = of_match_device(kpss_xcc_match_table, &pdev->dev);
@@ -55,24 +55,15 @@ static int kpss_xcc_driver_probe(struct platform_device *pdev)
 		base += 0x28;
 	}
 
-	clk = clk_register_mux_table(&pdev->dev, name, aux_parents,
-				     ARRAY_SIZE(aux_parents), 0, base, 0, 0x3,
-				     0, aux_parent_map, NULL);
+	hw = __devm_clk_hw_register_mux(&pdev->dev, NULL, name, ARRAY_SIZE(aux_parents),
+					NULL, NULL, aux_parents, 0, base, 0, 0x3,
+					0, aux_parent_map, NULL);
 
-	platform_set_drvdata(pdev, clk);
-
-	return PTR_ERR_OR_ZERO(clk);
-}
-
-static int kpss_xcc_driver_remove(struct platform_device *pdev)
-{
-	clk_unregister_mux(platform_get_drvdata(pdev));
-	return 0;
+	return PTR_ERR_OR_ZERO(hw);
 }
 
 static struct platform_driver kpss_xcc_driver = {
 	.probe = kpss_xcc_driver_probe,
-	.remove = kpss_xcc_driver_remove,
 	.driver = {
 		.name = "kpss-xcc",
 		.of_match_table = kpss_xcc_match_table,
-- 
2.34.1

[PATCH 06/16] clk: qcom: clk-krait: unlock spin after mux completion

[Ansuel Smith]

Unlock spinlock after the mux switch is completed to prevent any corner
case of mux request while the switch still needs to be done.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/clk-krait.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
index 59f1af415b58..e447fcc3806d 100644
--- a/drivers/clk/qcom/clk-krait.c
+++ b/drivers/clk/qcom/clk-krait.c
@@ -32,11 +32,12 @@ static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
 		regval |= (sel & mux->mask) << (mux->shift + LPL_SHIFT);
 	}
 	krait_set_l2_indirect_reg(mux->offset, regval);
-	spin_unlock_irqrestore(&krait_clock_reg_lock, flags);
 
 	/* Wait for switch to complete. */
 	mb();
 	udelay(1);
+
+	spin_unlock_irqrestore(&krait_clock_reg_lock, flags);
 }
 
 static int krait_mux_set_parent(struct clk_hw *hw, u8 index)
-- 
2.34.1

[PATCH 07/16] clk: qcom: clk-krait: add hw_parent check for div2_round_rate

[Ansuel Smith]

Check if hw_parent is present before calculating the round_rate to
prevent kernel panic.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/clk-krait.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
index e447fcc3806d..d8af281eba0e 100644
--- a/drivers/clk/qcom/clk-krait.c
+++ b/drivers/clk/qcom/clk-krait.c
@@ -80,7 +80,12 @@ EXPORT_SYMBOL_GPL(krait_mux_clk_ops);
 static long krait_div2_round_rate(struct clk_hw *hw, unsigned long rate,
 				  unsigned long *parent_rate)
 {
-	*parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), rate * 2);
+	struct clk_hw *hw_parent = clk_hw_get_parent(hw);
+
+	if (!hw_parent)
+		return -1;
+
+	*parent_rate = clk_hw_round_rate(hw_parent, rate * 2);
 	return DIV_ROUND_UP(*parent_rate, 2);
 }
 
-- 
2.34.1

[PATCH 08/16] clk: qcom: krait-cc: convert to parent_data API

[Ansuel Smith]

Modernize the krait-cc driver to parent-data API and refactor to drop
any use of clk_names. From Documentation all the required clocks should
be declared in DTS so fw_name can be correctly used to get the parents
for all the muxes. Name is also declared to save compatibility with old
implementation. Also fix the parent order of the sec_mux that was wrong
and incorrectly report the wrong safe parent if it's not hardcoded.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/krait-cc.c | 126 +++++++++++++++++++-----------------
 1 file changed, 66 insertions(+), 60 deletions(-)

diff --git a/drivers/clk/qcom/krait-cc.c b/drivers/clk/qcom/krait-cc.c
index 4d4b657d33c3..645ad9e8dd73 100644
--- a/drivers/clk/qcom/krait-cc.c
+++ b/drivers/clk/qcom/krait-cc.c
@@ -69,21 +69,22 @@ static int krait_notifier_register(struct device *dev, struct clk *clk,
 	return ret;
 }
 
-static int
+static struct clk *
 krait_add_div(struct device *dev, int id, const char *s, unsigned int offset)
 {
 	struct krait_div2_clk *div;
+	static struct clk_parent_data p_data[1];
 	struct clk_init_data init = {
-		.num_parents = 1,
+		.num_parents = ARRAY_SIZE(p_data),
 		.ops = &krait_div2_clk_ops,
 		.flags = CLK_SET_RATE_PARENT,
 	};
-	const char *p_names[1];
 	struct clk *clk;
+	char *parent_name;
 
 	div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL);
 	if (!div)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	div->width = 2;
 	div->shift = 6;
@@ -93,43 +94,49 @@ krait_add_div(struct device *dev, int id, const char *s, unsigned int offset)
 
 	init.name = kasprintf(GFP_KERNEL, "hfpll%s_div", s);
 	if (!init.name)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
-	init.parent_names = p_names;
-	p_names[0] = kasprintf(GFP_KERNEL, "hfpll%s", s);
-	if (!p_names[0]) {
-		kfree(init.name);
-		return -ENOMEM;
+	init.parent_data = p_data;
+	parent_name = kasprintf(GFP_KERNEL, "hfpll%s", s);
+	if (!parent_name) {
+		clk = ERR_PTR(-ENOMEM);
+		goto err_parent_name;
 	}
 
+	p_data[0].fw_name = parent_name;
+	p_data[0].name = parent_name;
+
 	clk = devm_clk_register(dev, &div->hw);
-	kfree(p_names[0]);
+
+	kfree(parent_name);
+err_parent_name:
 	kfree(init.name);
 
-	return PTR_ERR_OR_ZERO(clk);
+	return clk;
 }
 
-static int
+static struct clk *
 krait_add_sec_mux(struct device *dev, int id, const char *s,
 		  unsigned int offset, bool unique_aux)
 {
 	int ret;
 	struct krait_mux_clk *mux;
-	static const char *sec_mux_list[] = {
-		"acpu_aux",
-		"qsb",
+	static struct clk_parent_data sec_mux_list[2] = {
+		{ .name = "qsb", .fw_name = "qsb" },
+		{},
 	};
 	struct clk_init_data init = {
-		.parent_names = sec_mux_list,
+		.parent_data = sec_mux_list,
 		.num_parents = ARRAY_SIZE(sec_mux_list),
 		.ops = &krait_mux_clk_ops,
 		.flags = CLK_SET_RATE_PARENT,
 	};
 	struct clk *clk;
+	char *parent_name;
 
 	mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
 	if (!mux)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	mux->offset = offset;
 	mux->lpl = id >= 0;
@@ -141,44 +148,51 @@ krait_add_sec_mux(struct device *dev, int id, const char *s,
 
 	init.name = kasprintf(GFP_KERNEL, "krait%s_sec_mux", s);
 	if (!init.name)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	if (unique_aux) {
-		sec_mux_list[0] = kasprintf(GFP_KERNEL, "acpu%s_aux", s);
-		if (!sec_mux_list[0]) {
+		parent_name = kasprintf(GFP_KERNEL, "acpu%s_aux", s);
+		if (!parent_name) {
 			clk = ERR_PTR(-ENOMEM);
 			goto err_aux;
 		}
+		sec_mux_list[1].fw_name = parent_name;
+		sec_mux_list[1].name = parent_name;
+	} else {
+		sec_mux_list[1].name = "apu_aux";
 	}
 
 	clk = devm_clk_register(dev, &mux->hw);
+	if (IS_ERR(clk))
+		goto err_clk;
 
 	ret = krait_notifier_register(dev, clk, mux);
 	if (ret)
-		goto unique_aux;
+		clk = ERR_PTR(ret);
 
-unique_aux:
+err_clk:
 	if (unique_aux)
-		kfree(sec_mux_list[0]);
+		kfree(parent_name);
 err_aux:
 	kfree(init.name);
-	return PTR_ERR_OR_ZERO(clk);
+	return clk;
 }
 
 static struct clk *
-krait_add_pri_mux(struct device *dev, int id, const char *s,
-		  unsigned int offset)
+krait_add_pri_mux(struct device *dev, struct clk *hfpll_div, struct clk *sec_mux,
+		  int id, const char *s, unsigned int offset)
 {
 	int ret;
 	struct krait_mux_clk *mux;
-	const char *p_names[3];
+	static struct clk_parent_data p_data[3];
 	struct clk_init_data init = {
-		.parent_names = p_names,
-		.num_parents = ARRAY_SIZE(p_names),
+		.parent_data = p_data,
+		.num_parents = ARRAY_SIZE(p_data),
 		.ops = &krait_mux_clk_ops,
 		.flags = CLK_SET_RATE_PARENT,
 	};
 	struct clk *clk;
+	char *hfpll_name;
 
 	mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
 	if (!mux)
@@ -196,36 +210,29 @@ krait_add_pri_mux(struct device *dev, int id, const char *s,
 	if (!init.name)
 		return ERR_PTR(-ENOMEM);
 
-	p_names[0] = kasprintf(GFP_KERNEL, "hfpll%s", s);
-	if (!p_names[0]) {
+	hfpll_name = kasprintf(GFP_KERNEL, "hfpll%s", s);
+	if (!hfpll_name) {
 		clk = ERR_PTR(-ENOMEM);
-		goto err_p0;
+		goto err_hfpll;
 	}
 
-	p_names[1] = kasprintf(GFP_KERNEL, "hfpll%s_div", s);
-	if (!p_names[1]) {
-		clk = ERR_PTR(-ENOMEM);
-		goto err_p1;
-	}
+	p_data[0].fw_name = hfpll_name;
+	p_data[0].name = hfpll_name;
 
-	p_names[2] = kasprintf(GFP_KERNEL, "krait%s_sec_mux", s);
-	if (!p_names[2]) {
-		clk = ERR_PTR(-ENOMEM);
-		goto err_p2;
-	}
+	p_data[1].hw = __clk_get_hw(hfpll_div);
+	p_data[2].hw = __clk_get_hw(sec_mux);
 
 	clk = devm_clk_register(dev, &mux->hw);
+	if (IS_ERR(clk))
+		goto err_clk;
 
 	ret = krait_notifier_register(dev, clk, mux);
 	if (ret)
-		goto err_p3;
-err_p3:
-	kfree(p_names[2]);
-err_p2:
-	kfree(p_names[1]);
-err_p1:
-	kfree(p_names[0]);
-err_p0:
+		clk = ERR_PTR(ret);
+
+err_clk:
+	kfree(hfpll_name);
+err_hfpll:
 	kfree(init.name);
 	return clk;
 }
@@ -233,11 +240,10 @@ krait_add_pri_mux(struct device *dev, int id, const char *s,
 /* id < 0 for L2, otherwise id == physical CPU number */
 static struct clk *krait_add_clks(struct device *dev, int id, bool unique_aux)
 {
-	int ret;
 	unsigned int offset;
 	void *p = NULL;
 	const char *s;
-	struct clk *clk;
+	struct clk *hfpll_div, *sec_mux, *clk;
 
 	if (id >= 0) {
 		offset = 0x4501 + (0x1000 * id);
@@ -249,19 +255,19 @@ static struct clk *krait_add_clks(struct device *dev, int id, bool unique_aux)
 		s = "_l2";
 	}
 
-	ret = krait_add_div(dev, id, s, offset);
-	if (ret) {
-		clk = ERR_PTR(ret);
+	hfpll_div = krait_add_div(dev, id, s, offset);
+	if (IS_ERR(hfpll_div)) {
+		clk = hfpll_div;
 		goto err;
 	}
 
-	ret = krait_add_sec_mux(dev, id, s, offset, unique_aux);
-	if (ret) {
-		clk = ERR_PTR(ret);
+	sec_mux = krait_add_sec_mux(dev, id, s, offset, unique_aux);
+	if (IS_ERR(sec_mux)) {
+		clk = sec_mux;
 		goto err;
 	}
 
-	clk = krait_add_pri_mux(dev, id, s, offset);
+	clk = krait_add_pri_mux(dev, hfpll_div, sec_mux, id, s, offset);
 err:
 	kfree(p);
 	return clk;
-- 
2.34.1

[PATCH 09/16] clk: qcom: krait-cc: drop pr_info and register qsb only if needed

[Ansuel Smith]

Drop pr_info and change them to dev_info.
Register qsb fixed clk only if it's not declared in DTS.
Also reorganize variable order.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/krait-cc.c | 42 +++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/clk/qcom/krait-cc.c b/drivers/clk/qcom/krait-cc.c
index 645ad9e8dd73..50352ff0ac67 100644
--- a/drivers/clk/qcom/krait-cc.c
+++ b/drivers/clk/qcom/krait-cc.c
@@ -34,9 +34,10 @@ static int krait_notifier_cb(struct notifier_block *nb,
 			     unsigned long event,
 			     void *data)
 {
-	int ret = 0;
 	struct krait_mux_clk *mux = container_of(nb, struct krait_mux_clk,
 						 clk_nb);
+	int ret = 0;
+
 	/* Switch to safe parent */
 	if (event == PRE_RATE_CHANGE) {
 		mux->old_index = krait_mux_clk_ops.get_parent(&mux->hw);
@@ -59,7 +60,7 @@ static int krait_notifier_cb(struct notifier_block *nb,
 static int krait_notifier_register(struct device *dev, struct clk *clk,
 				   struct krait_mux_clk *mux)
 {
-	int ret = 0;
+	int ret;
 
 	mux->clk_nb.notifier_call = krait_notifier_cb;
 	ret = clk_notifier_register(clk, &mux->clk_nb);
@@ -72,15 +73,15 @@ static int krait_notifier_register(struct device *dev, struct clk *clk,
 static struct clk *
 krait_add_div(struct device *dev, int id, const char *s, unsigned int offset)
 {
-	struct krait_div2_clk *div;
 	static struct clk_parent_data p_data[1];
 	struct clk_init_data init = {
 		.num_parents = ARRAY_SIZE(p_data),
 		.ops = &krait_div2_clk_ops,
 		.flags = CLK_SET_RATE_PARENT,
 	};
-	struct clk *clk;
+	struct krait_div2_clk *div;
 	char *parent_name;
+	struct clk *clk;
 
 	div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL);
 	if (!div)
@@ -119,8 +120,6 @@ static struct clk *
 krait_add_sec_mux(struct device *dev, int id, const char *s,
 		  unsigned int offset, bool unique_aux)
 {
-	int ret;
-	struct krait_mux_clk *mux;
 	static struct clk_parent_data sec_mux_list[2] = {
 		{ .name = "qsb", .fw_name = "qsb" },
 		{},
@@ -131,8 +130,10 @@ krait_add_sec_mux(struct device *dev, int id, const char *s,
 		.ops = &krait_mux_clk_ops,
 		.flags = CLK_SET_RATE_PARENT,
 	};
-	struct clk *clk;
+	struct krait_mux_clk *mux;
 	char *parent_name;
+	struct clk *clk;
+	int ret;
 
 	mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
 	if (!mux)
@@ -182,8 +183,6 @@ static struct clk *
 krait_add_pri_mux(struct device *dev, struct clk *hfpll_div, struct clk *sec_mux,
 		  int id, const char *s, unsigned int offset)
 {
-	int ret;
-	struct krait_mux_clk *mux;
 	static struct clk_parent_data p_data[3];
 	struct clk_init_data init = {
 		.parent_data = p_data,
@@ -191,8 +190,10 @@ krait_add_pri_mux(struct device *dev, struct clk *hfpll_div, struct clk *sec_mux
 		.ops = &krait_mux_clk_ops,
 		.flags = CLK_SET_RATE_PARENT,
 	};
-	struct clk *clk;
+	struct krait_mux_clk *mux;
 	char *hfpll_name;
+	struct clk *clk;
+	int ret;
 
 	mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
 	if (!mux)
@@ -240,10 +241,10 @@ krait_add_pri_mux(struct device *dev, struct clk *hfpll_div, struct clk *sec_mux
 /* id < 0 for L2, otherwise id == physical CPU number */
 static struct clk *krait_add_clks(struct device *dev, int id, bool unique_aux)
 {
+	struct clk *hfpll_div, *sec_mux, *clk;
 	unsigned int offset;
 	void *p = NULL;
 	const char *s;
-	struct clk *hfpll_div, *sec_mux, *clk;
 
 	if (id >= 0) {
 		offset = 0x4501 + (0x1000 * id);
@@ -295,20 +296,21 @@ MODULE_DEVICE_TABLE(of, krait_cc_match_table);
 
 static int krait_cc_probe(struct platform_device *pdev)
 {
+	unsigned long cur_rate, aux_rate;
+	struct clk *l2_pri_mux_clk, *clk;
 	struct device *dev = &pdev->dev;
 	const struct of_device_id *id;
-	unsigned long cur_rate, aux_rate;
-	int cpu;
-	struct clk *clk;
 	struct clk **clks;
-	struct clk *l2_pri_mux_clk;
+	int cpu;
 
 	id = of_match_device(krait_cc_match_table, dev);
 	if (!id)
 		return -ENODEV;
 
 	/* Rate is 1 because 0 causes problems for __clk_mux_determine_rate */
-	clk = clk_register_fixed_rate(dev, "qsb", NULL, 0, 1);
+	if (IS_ERR(clk_get(dev, "qsb")))
+		clk = clk_register_fixed_rate(dev, "qsb", NULL, 0, 1);
+
 	if (IS_ERR(clk))
 		return PTR_ERR(clk);
 
@@ -363,25 +365,25 @@ static int krait_cc_probe(struct platform_device *pdev)
 	cur_rate = clk_get_rate(l2_pri_mux_clk);
 	aux_rate = 384000000;
 	if (cur_rate == 1) {
-		pr_info("L2 @ QSB rate. Forcing new rate.\n");
+		dev_info(dev, "L2 @ QSB rate. Forcing new rate.\n");
 		cur_rate = aux_rate;
 	}
 	clk_set_rate(l2_pri_mux_clk, aux_rate);
 	clk_set_rate(l2_pri_mux_clk, 2);
 	clk_set_rate(l2_pri_mux_clk, cur_rate);
-	pr_info("L2 @ %lu KHz\n", clk_get_rate(l2_pri_mux_clk) / 1000);
+	dev_info(dev, "L2 @ %lu KHz\n", clk_get_rate(l2_pri_mux_clk) / 1000);
 	for_each_possible_cpu(cpu) {
 		clk = clks[cpu];
 		cur_rate = clk_get_rate(clk);
 		if (cur_rate == 1) {
-			pr_info("CPU%d @ QSB rate. Forcing new rate.\n", cpu);
+			dev_info(dev, "CPU%d @ QSB rate. Forcing new rate.\n", cpu);
 			cur_rate = aux_rate;
 		}
 
 		clk_set_rate(clk, aux_rate);
 		clk_set_rate(clk, 2);
 		clk_set_rate(clk, cur_rate);
-		pr_info("CPU%d @ %lu KHz\n", cpu, clk_get_rate(clk) / 1000);
+		dev_info(dev, "CPU%d @ %lu KHz\n", cpu, clk_get_rate(clk) / 1000);
 	}
 
 	of_clk_add_provider(dev->of_node, krait_of_get, clks);
-- 
2.34.1

[PATCH 10/16] clk: qcom: krait-cc: drop hardcoded safe_sel

[Ansuel Smith]

Drop hardcoded safe_sel definition and use helper to correctly calculate
it. We assume qsb clk is always present as it should be declared in DTS
per Documentation and in the absence of that, it's declared as a fixed
clk.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/krait-cc.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/qcom/krait-cc.c b/drivers/clk/qcom/krait-cc.c
index 50352ff0ac67..6530f10a546f 100644
--- a/drivers/clk/qcom/krait-cc.c
+++ b/drivers/clk/qcom/krait-cc.c
@@ -26,6 +26,13 @@ static unsigned int pri_mux_map[] = {
 	0,
 };
 
+static u8 krait_get_mux_sel(struct krait_mux_clk *mux, struct clk *safe_clk)
+{
+	struct clk_hw *safe_hw = __clk_get_hw(safe_clk);
+
+	return clk_hw_get_parent_index(&mux->hw, safe_hw);
+}
+
 /*
  * Notifier function for switching the muxes to safe parent
  * while the hfpll is getting reprogrammed.
@@ -117,8 +124,8 @@ krait_add_div(struct device *dev, int id, const char *s, unsigned int offset)
 }
 
 static struct clk *
-krait_add_sec_mux(struct device *dev, int id, const char *s,
-		  unsigned int offset, bool unique_aux)
+krait_add_sec_mux(struct device *dev, struct clk *qsb, int id,
+		  const char *s, unsigned int offset, bool unique_aux)
 {
 	static struct clk_parent_data sec_mux_list[2] = {
 		{ .name = "qsb", .fw_name = "qsb" },
@@ -145,7 +152,6 @@ krait_add_sec_mux(struct device *dev, int id, const char *s,
 	mux->shift = 2;
 	mux->parent_map = sec_mux_map;
 	mux->hw.init = &init;
-	mux->safe_sel = 0;
 
 	init.name = kasprintf(GFP_KERNEL, "krait%s_sec_mux", s);
 	if (!init.name)
@@ -167,6 +173,7 @@ krait_add_sec_mux(struct device *dev, int id, const char *s,
 	if (IS_ERR(clk))
 		goto err_clk;
 
+	mux->safe_sel = krait_get_mux_sel(mux, qsb);
 	ret = krait_notifier_register(dev, clk, mux);
 	if (ret)
 		clk = ERR_PTR(ret);
@@ -205,7 +212,6 @@ krait_add_pri_mux(struct device *dev, struct clk *hfpll_div, struct clk *sec_mux
 	mux->lpl = id >= 0;
 	mux->parent_map = pri_mux_map;
 	mux->hw.init = &init;
-	mux->safe_sel = 2;
 
 	init.name = kasprintf(GFP_KERNEL, "krait%s_pri_mux", s);
 	if (!init.name)
@@ -227,6 +233,7 @@ krait_add_pri_mux(struct device *dev, struct clk *hfpll_div, struct clk *sec_mux
 	if (IS_ERR(clk))
 		goto err_clk;
 
+	mux->safe_sel = krait_get_mux_sel(mux, sec_mux);
 	ret = krait_notifier_register(dev, clk, mux);
 	if (ret)
 		clk = ERR_PTR(ret);
@@ -239,7 +246,9 @@ krait_add_pri_mux(struct device *dev, struct clk *hfpll_div, struct clk *sec_mux
 }
 
 /* id < 0 for L2, otherwise id == physical CPU number */
-static struct clk *krait_add_clks(struct device *dev, int id, bool unique_aux)
+static struct clk *
+krait_add_clks(struct device *dev, struct clk *qsb, int id,
+	       bool unique_aux)
 {
 	struct clk *hfpll_div, *sec_mux, *clk;
 	unsigned int offset;
@@ -262,7 +271,7 @@ static struct clk *krait_add_clks(struct device *dev, int id, bool unique_aux)
 		goto err;
 	}
 
-	sec_mux = krait_add_sec_mux(dev, id, s, offset, unique_aux);
+	sec_mux = krait_add_sec_mux(dev, qsb, id, s, offset, unique_aux);
 	if (IS_ERR(sec_mux)) {
 		clk = sec_mux;
 		goto err;
@@ -296,8 +305,8 @@ MODULE_DEVICE_TABLE(of, krait_cc_match_table);
 
 static int krait_cc_probe(struct platform_device *pdev)
 {
+	struct clk *l2_pri_mux_clk, *qsb, *clk;
 	unsigned long cur_rate, aux_rate;
-	struct clk *l2_pri_mux_clk, *clk;
 	struct device *dev = &pdev->dev;
 	const struct of_device_id *id;
 	struct clk **clks;
@@ -308,11 +317,12 @@ static int krait_cc_probe(struct platform_device *pdev)
 		return -ENODEV;
 
 	/* Rate is 1 because 0 causes problems for __clk_mux_determine_rate */
-	if (IS_ERR(clk_get(dev, "qsb")))
-		clk = clk_register_fixed_rate(dev, "qsb", NULL, 0, 1);
+	qsb = clk_get(dev, "qsb");
+	if (IS_ERR(qsb))
+		qsb = clk_register_fixed_rate(dev, "qsb", NULL, 0, 1);
 
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	if (IS_ERR(qsb))
+		return PTR_ERR(qsb);
 
 	if (!id->data) {
 		clk = clk_register_fixed_factor(dev, "acpu_aux",
@@ -327,13 +337,13 @@ static int krait_cc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	for_each_possible_cpu(cpu) {
-		clk = krait_add_clks(dev, cpu, id->data);
+		clk = krait_add_clks(dev, qsb, cpu, id->data);
 		if (IS_ERR(clk))
 			return PTR_ERR(clk);
 		clks[cpu] = clk;
 	}
 
-	l2_pri_mux_clk = krait_add_clks(dev, -1, id->data);
+	l2_pri_mux_clk = krait_add_clks(dev, qsb, -1, id->data);
 	if (IS_ERR(l2_pri_mux_clk))
 		return PTR_ERR(l2_pri_mux_clk);
 	clks[4] = l2_pri_mux_clk;
-- 
2.34.1

[PATCH 11/16] clk: qcom: krait-cc: force sec_mux to QSB

[Ansuel Smith]

Now that we have converted every driver to parent_data, it was
notice that the bootloader can't really leave the system in a
strange state where l2 or the cpu0/1 can be sourced in a number of ways
for example cpu1 sourcing out of qsb, l2 sourcing out of pxo.
To correctly reset the mux and the HFPLL force the sec_mux to QSB.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/krait-cc.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/krait-cc.c b/drivers/clk/qcom/krait-cc.c
index 6530f10a546f..1bdc89c097e6 100644
--- a/drivers/clk/qcom/krait-cc.c
+++ b/drivers/clk/qcom/krait-cc.c
@@ -15,6 +15,8 @@
 
 #include "clk-krait.h"
 
+#define QSB_RATE	1
+
 static unsigned int sec_mux_map[] = {
 	2,
 	0,
@@ -178,6 +180,12 @@ krait_add_sec_mux(struct device *dev, struct clk *qsb, int id,
 	if (ret)
 		clk = ERR_PTR(ret);
 
+	/* Force the sec_mux to be set to QSB rate.
+	 * This is needed to correctly set the parents and
+	 * to later reset mux and HFPLL to a known freq.
+	 */
+	clk_set_rate(clk, QSB_RATE);
+
 err_clk:
 	if (unique_aux)
 		kfree(parent_name);
@@ -374,7 +382,7 @@ static int krait_cc_probe(struct platform_device *pdev)
 	 */
 	cur_rate = clk_get_rate(l2_pri_mux_clk);
 	aux_rate = 384000000;
-	if (cur_rate == 1) {
+	if (cur_rate == QSB_RATE) {
 		dev_info(dev, "L2 @ QSB rate. Forcing new rate.\n");
 		cur_rate = aux_rate;
 	}
@@ -385,7 +393,7 @@ static int krait_cc_probe(struct platform_device *pdev)
 	for_each_possible_cpu(cpu) {
 		clk = clks[cpu];
 		cur_rate = clk_get_rate(clk);
-		if (cur_rate == 1) {
+		if (cur_rate == QSB_RATE) {
 			dev_info(dev, "CPU%d @ QSB rate. Forcing new rate.\n", cpu);
 			cur_rate = aux_rate;
 		}
-- 
2.34.1

[PATCH 12/16] clk: qcom: clk-krait: add 8064 errata workaround

[Ansuel Smith]

Add 8064 errata workaround where the sec_src clock gating needs to be
disabled during switching. To enable this set disable_sec_src_gating in
the mux struct.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/clk-krait.c | 16 ++++++++++++++++
 drivers/clk/qcom/clk-krait.h |  1 +
 drivers/clk/qcom/krait-cc.c  |  1 +
 3 files changed, 18 insertions(+)

diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
index d8af281eba0e..82fe7031e1f4 100644
--- a/drivers/clk/qcom/clk-krait.c
+++ b/drivers/clk/qcom/clk-krait.c
@@ -18,13 +18,23 @@
 static DEFINE_SPINLOCK(krait_clock_reg_lock);
 
 #define LPL_SHIFT	8
+#define SECCLKAGD	BIT(4)
+
 static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
 {
 	unsigned long flags;
 	u32 regval;
 
 	spin_lock_irqsave(&krait_clock_reg_lock, flags);
+
 	regval = krait_get_l2_indirect_reg(mux->offset);
+
+	/* 8064 Errata: disable sec_src clock gating during switch. */
+	if (mux->disable_sec_src_gating) {
+		regval |= SECCLKAGD;
+		krait_set_l2_indirect_reg(mux->offset, regval);
+	}
+
 	regval &= ~(mux->mask << mux->shift);
 	regval |= (sel & mux->mask) << mux->shift;
 	if (mux->lpl) {
@@ -33,6 +43,12 @@ static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
 	}
 	krait_set_l2_indirect_reg(mux->offset, regval);
 
+	/* 8064 Errata: re-enabled sec_src clock gating. */
+	if (mux->disable_sec_src_gating) {
+		regval &= ~SECCLKAGD;
+		krait_set_l2_indirect_reg(mux->offset, regval);
+	}
+
 	/* Wait for switch to complete. */
 	mb();
 	udelay(1);
diff --git a/drivers/clk/qcom/clk-krait.h b/drivers/clk/qcom/clk-krait.h
index 9120bd2f5297..f930538c539e 100644
--- a/drivers/clk/qcom/clk-krait.h
+++ b/drivers/clk/qcom/clk-krait.h
@@ -15,6 +15,7 @@ struct krait_mux_clk {
 	u8		safe_sel;
 	u8		old_index;
 	bool		reparent;
+	bool		disable_sec_src_gating;
 
 	struct clk_hw	hw;
 	struct notifier_block   clk_nb;
diff --git a/drivers/clk/qcom/krait-cc.c b/drivers/clk/qcom/krait-cc.c
index 1bdc89c097e6..533a770332be 100644
--- a/drivers/clk/qcom/krait-cc.c
+++ b/drivers/clk/qcom/krait-cc.c
@@ -154,6 +154,7 @@ krait_add_sec_mux(struct device *dev, struct clk *qsb, int id,
 	mux->shift = 2;
 	mux->parent_map = sec_mux_map;
 	mux->hw.init = &init;
+	mux->disable_sec_src_gating = true;
 
 	init.name = kasprintf(GFP_KERNEL, "krait%s_sec_mux", s);
 	if (!init.name)
-- 
2.34.1

[PATCH 13/16] clk: qcom: clk-krait: add enable disable ops

[Ansuel Smith]

Add enable/disable ops for krait mux. On disable the mux is set to the
safe selection.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/clk-krait.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
index 82fe7031e1f4..fc277fe3841c 100644
--- a/drivers/clk/qcom/clk-krait.c
+++ b/drivers/clk/qcom/clk-krait.c
@@ -85,7 +85,25 @@ static u8 krait_mux_get_parent(struct clk_hw *hw)
 	return clk_mux_val_to_index(hw, mux->parent_map, 0, sel);
 }
 
+static int krait_mux_enable(struct clk_hw *hw)
+{
+	struct krait_mux_clk *mux = to_krait_mux_clk(hw);
+
+	__krait_mux_set_sel(mux, mux->en_mask);
+
+	return 0;
+}
+
+static void krait_mux_disable(struct clk_hw *hw)
+{
+	struct krait_mux_clk *mux = to_krait_mux_clk(hw);
+
+	__krait_mux_set_sel(mux, mux->safe_sel);
+}
+
 const struct clk_ops krait_mux_clk_ops = {
+	.enable = krait_mux_enable,
+	.disable = krait_mux_disable,
 	.set_parent = krait_mux_set_parent,
 	.get_parent = krait_mux_get_parent,
 	.determine_rate = __clk_mux_determine_rate_closest,
-- 
2.34.1

[PATCH 14/16] dt-bindings: clock: Convert qcom,krait-cc to yaml

[Ansuel Smith]

Convert qcom,krait-cc to yaml and add missing l2 clocks and names
definiton.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 .../bindings/clock/qcom,krait-cc.txt          | 34 ----------
 .../bindings/clock/qcom,krait-cc.yaml         | 63 +++++++++++++++++++
 2 files changed, 63 insertions(+), 34 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/qcom,krait-cc.txt
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,krait-cc.yaml

diff --git a/Documentation/devicetree/bindings/clock/qcom,krait-cc.txt b/Documentation/devicetree/bindings/clock/qcom,krait-cc.txt
deleted file mode 100644
index 030ba60dab08..000000000000
--- a/Documentation/devicetree/bindings/clock/qcom,krait-cc.txt
+++ /dev/null
@@ -1,34 +0,0 @@
-Krait Clock Controller
-
-PROPERTIES
-
-- compatible:
-	Usage: required
-	Value type: <string>
-	Definition: must be one of:
-			"qcom,krait-cc-v1"
-			"qcom,krait-cc-v2"
-
-- #clock-cells:
-	Usage: required
-	Value type: <u32>
-	Definition: must be 1
-
-- clocks:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: reference to the clock parents of hfpll, secondary muxes.
-
-- clock-names:
-	Usage: required
-	Value type: <stringlist>
-	Definition: must be "hfpll0", "hfpll1", "acpu0_aux", "acpu1_aux", "qsb".
-
-Example:
-
-	kraitcc: clock-controller {
-		compatible = "qcom,krait-cc-v1";
-		clocks = <&hfpll0>, <&hfpll1>, <&acpu0_aux>, <&acpu1_aux>, <qsb>;
-		clock-names = "hfpll0", "hfpll1", "acpu0_aux", "acpu1_aux", "qsb";
-		#clock-cells = <1>;
-	};
diff --git a/Documentation/devicetree/bindings/clock/qcom,krait-cc.yaml b/Documentation/devicetree/bindings/clock/qcom,krait-cc.yaml
new file mode 100644
index 000000000000..f89b70ab01ae
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,krait-cc.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,krait-cc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Krait Clock Controller
+
+maintainers:
+  - Ansuel Smith <ansuelsmth@gmail.com>
+
+description: |
+  Qualcomm Krait Clock Controller used to correctly scale the CPU and the L2
+  rates.
+
+properties:
+  compatible:
+    enum:
+      - qcom,krait-cc-v1
+      - qcom,krait-cc-v2
+
+  clocks:
+    items:
+      - description: phandle to hfpll for CPU0 mux
+      - description: phandle to hfpll for CPU1 mux
+      - description: phandle to hfpll for L2 mux
+      - description: phandle to CPU0 aux clock
+      - description: phandle to CPU1 aux clock
+      - description: phandle to L2 aux clock
+      - description: phandle to QSB fixed clk
+
+  clock-names:
+    items:
+      - const: hfpll0
+      - const: hfpll1
+      - const: hfpll_l2
+      - const: acpu0_aux
+      - const: acpu1_aux
+      - const: acpu_l2_aux
+      - const: qsb
+
+  '#clock-cells':
+    const: 1
+
+required:
+  - compatible
+  - clocks
+  - clock-names
+  - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    clock-controller {
+      compatible = "qcom,krait-cc-v1";
+      clocks = <&hfpll0>, <&hfpll1>, <&hfpll_l2>,
+               <&acpu0_aux>, <&acpu1_aux>, <&acpu_l2_aux>, <&qsb>;
+      clock-names = "hfpll0", "hfpll1", "hfpll_l2",
+                    "acpu0_aux", "acpu1_aux", "acpu_l2_aux", "qsb";
+      #clock-cells = <1>;
+    };
+...
-- 
2.34.1

[PATCH 15/16] dts: qcom-ipq8064: add missing krait-cc compatible and clocks

[Ansuel Smith]

Add missing krait-cc clock-controller and define missing aux clock for
CPUs. Also change phandle for l2cc node to point to pxo_board instead
of gcc PXO_SRC.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 arch/arm/boot/dts/qcom-ipq8064.dtsi | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
index 996f4458d9fc..888f17d64283 100644
--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
@@ -468,11 +468,19 @@ IRQ_TYPE_EDGE_RISING)>,
 		acc0: clock-controller@2088000 {
 			compatible = "qcom,kpss-acc-v1";
 			reg = <0x02088000 0x1000>, <0x02008000 0x1000>;
+			clock-output-names = "acpu0_aux";
+			clocks = <&gcc PLL8_VOTE>, <&pxo_board>;
+			clock-names = "pll8_vote", "pxo";
+			#clock-cells = <0>;
 		};
 
 		acc1: clock-controller@2098000 {
 			compatible = "qcom,kpss-acc-v1";
 			reg = <0x02098000 0x1000>, <0x02008000 0x1000>;
+			clock-output-names = "acpu1_aux";
+			clocks = <&gcc PLL8_VOTE>, <&pxo_board>;
+			clock-names = "pll8_vote", "pxo";
+			#clock-cells = <0>;
 		};
 
 		adm_dma: dma-controller@18300000 {
@@ -782,11 +790,21 @@ tcsr: syscon@1a400000 {
 		l2cc: clock-controller@2011000 {
 			compatible = "qcom,kpss-gcc", "syscon";
 			reg = <0x2011000 0x1000>;
-			clocks = <&gcc PLL8_VOTE>, <&gcc PXO_SRC>;
+			clocks = <&gcc PLL8_VOTE>, <&pxo_board>;
 			clock-names = "pll8_vote", "pxo";
 			clock-output-names = "acpu_l2_aux";
 		};
 
+		kraitcc: clock-controller {
+			compatible = "qcom,krait-cc-v1";
+			clocks = <&gcc PLL9>, <&gcc PLL10>, <&gcc PLL12>,
+				 <&acc0>, <&acc1>, <&l2cc>, <&qsb>;
+			clock-names = "hfpll0", "hfpll1", "hfpll_l2",
+				      "acpu0_aux", "acpu1_aux", "acpu_l2_aux",
+				      "qsb";
+			#clock-cells = <1>;
+		};
+
 		lcc: clock-controller@28000000 {
 			compatible = "qcom,lcc-ipq8064";
 			reg = <0x28000000 0x1000>;
-- 
2.34.1

[PATCH 16/16] dt-bindings: arm: msm: Convert kpss driver Documentation to yaml

[Ansuel Smith]

Convert kpss-acc and kpss-gcc Documentation to yaml. Fix multiple
Documentation error and provide additional example for kpss-gcc-v2.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 .../bindings/arm/msm/qcom,kpss-acc.txt        | 49 ----------
 .../bindings/arm/msm/qcom,kpss-acc.yaml       | 97 +++++++++++++++++++
 .../bindings/arm/msm/qcom,kpss-gcc.txt        | 44 ---------
 .../bindings/arm/msm/qcom,kpss-gcc.yaml       | 62 ++++++++++++
 4 files changed, 159 insertions(+), 93 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
 create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.yaml
 delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
 create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.yaml

diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
deleted file mode 100644
index 7f696362a4a1..000000000000
--- a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
+++ /dev/null
@@ -1,49 +0,0 @@
-Krait Processor Sub-system (KPSS) Application Clock Controller (ACC)
-
-The KPSS ACC provides clock, power domain, and reset control to a Krait CPU.
-There is one ACC register region per CPU within the KPSS remapped region as
-well as an alias register region that remaps accesses to the ACC associated
-with the CPU accessing the region.
-
-PROPERTIES
-
-- compatible:
-	Usage: required
-	Value type: <string>
-	Definition: should be one of:
-			"qcom,kpss-acc-v1"
-			"qcom,kpss-acc-v2"
-
-- reg:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: the first element specifies the base address and size of
-		    the register region. An optional second element specifies
-		    the base address and size of the alias register region.
-
-- clocks:
-        Usage: required
-        Value type: <prop-encoded-array>
-        Definition: reference to the pll parents.
-
-- clock-names:
-        Usage: required
-        Value type: <stringlist>
-        Definition: must be "pll8_vote", "pxo".
-
-- clock-output-names:
-	Usage: optional
-	Value type: <string>
-	Definition: Name of the output clock. Typically acpuX_aux where X is a
-		    CPU number starting at 0.
-
-Example:
-
-	clock-controller@2088000 {
-		compatible = "qcom,kpss-acc-v2";
-		reg = <0x02088000 0x1000>,
-		      <0x02008000 0x1000>;
-		clocks = <&gcc PLL8_VOTE>, <&gcc PXO_SRC>;
-		clock-names = "pll8_vote", "pxo";
-		clock-output-names = "acpu0_aux";
-	};
diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.yaml
new file mode 100644
index 000000000000..6e8ef4f85eab
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/msm/qcom,kpss-acc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Krait Processor Sub-system (KPSS) Application Clock Controller (ACC)
+
+maintainers:
+  - Ansuel Smith <ansuelsmth@gmail.com>
+
+description: |
+  The KPSS ACC provides clock, power domain, and reset control to a Krait CPU.
+  There is one ACC register region per CPU within the KPSS remapped region as
+  well as an alias register region that remaps accesses to the ACC associated
+  with the CPU accessing the region.
+
+properties:
+  compatible:
+    enum:
+      - qcom,kpss-acc-v1
+      - qcom,kpss-acc-v2
+
+  reg:
+    items:
+      - description: Base address and size of the register region
+      - description: Optional base address and size of the alias register region
+
+  clocks:
+    items:
+      - description: phandle to pll8_vote
+      - description: phandle to pxo_board
+
+  clock-names:
+    items:
+      - const: pll8_vote
+      - const: pxo
+
+  clock-output-names:
+    description: Name of the aux clock. Krait can have at most 4 cpu.
+    enum:
+      - acpu0_aux
+      - acpu1_aux
+      - acpu2_aux
+      - acpu3_aux
+
+  '#clock-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,kpss-acc-v1
+      then:
+        required:
+          - clocks
+          - clock-names
+          - clock-output-names
+          - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-ipq806x.h>
+
+    clock-controller@2088000 {
+      compatible = "qcom,kpss-acc-v1";
+      reg = <0x02088000 0x1000>, <0x02008000 0x1000>;
+      clocks = <&gcc PLL8_VOTE>, <&pxo_board>;
+      clock-names = "pll8_vote", "pxo";
+      clock-output-names = "acpu0_aux";
+      #clock-cells = <0>;
+    };
+
+    clock-controller@2098000 {
+      compatible = "qcom,kpss-acc-v1";
+      reg = <0x02098000 0x1000>, <0x02008000 0x1000>;
+      clock-output-names = "acpu1_aux";
+      clocks = <&gcc PLL8_VOTE>, <&pxo_board>;
+      clock-names = "pll8_vote", "pxo";
+      #clock-cells = <0>;
+    };
+
+  - |
+    clock-controller@f9088000 {
+      compatible = "qcom,kpss-acc-v2";
+      reg = <0xf9088000 0x1000>,
+            <0xf9008000 0x1000>;
+    };
+...
diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
deleted file mode 100644
index e628758950e1..000000000000
--- a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
+++ /dev/null
@@ -1,44 +0,0 @@
-Krait Processor Sub-system (KPSS) Global Clock Controller (GCC)
-
-PROPERTIES
-
-- compatible:
-	Usage: required
-	Value type: <string>
-	Definition: should be one of the following. The generic compatible
-			"qcom,kpss-gcc" should also be included.
-			"qcom,kpss-gcc-ipq8064", "qcom,kpss-gcc"
-			"qcom,kpss-gcc-apq8064", "qcom,kpss-gcc"
-			"qcom,kpss-gcc-msm8974", "qcom,kpss-gcc"
-			"qcom,kpss-gcc-msm8960", "qcom,kpss-gcc"
-
-- reg:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: base address and size of the register region
-
-- clocks:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: reference to the pll parents.
-
-- clock-names:
-	Usage: required
-	Value type: <stringlist>
-	Definition: must be "pll8_vote", "pxo".
-
-- clock-output-names:
-	Usage: required
-	Value type: <string>
-	Definition: Name of the output clock. Typically acpu_l2_aux indicating
-		    an L2 cache auxiliary clock.
-
-Example:
-
-	l2cc: clock-controller@2011000 {
-		compatible = "qcom,kpss-gcc-ipq8064", "qcom,kpss-gcc";
-		reg = <0x2011000 0x1000>;
-		clocks = <&gcc PLL8_VOTE>, <&gcc PXO_SRC>;
-		clock-names = "pll8_vote", "pxo";
-		clock-output-names = "acpu_l2_aux";
-	};
diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.yaml
new file mode 100644
index 000000000000..3a2b54cc6a7c
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/msm/qcom,kpss-gcc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Krait Processor Sub-system (KPSS) Global Clock Controller (GCC)
+
+maintainers:
+  - Ansuel Smith <ansuelsmth@gmail.com>
+
+description: |
+  Krait Processor Sub-system (KPSS) Global Clock Controller (GCC). Used
+  to control L2 mux (in the current implementation).
+
+properties:
+  compatible:
+    const: qcom,kpss-gcc
+
+  reg:
+    items:
+      - description: Base address and size of the register region
+
+  clocks:
+    items:
+      - description: phandle to pll8_vote
+      - description: phandle to pxo_board
+
+  clock-names:
+    items:
+      - const: pll8_vote
+      - const: pxo
+
+  clock-output-names:
+    const: acpu_l2_aux
+
+  '#clock-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - clock-output-names
+  - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-ipq806x.h>
+
+    clock-controller@2011000 {
+      compatible = "qcom,kpss-gcc";
+      reg = <0x2011000 0x1000>;
+      clocks = <&gcc PLL8_VOTE>, <&pxo_board>;
+      clock-names = "pll8_vote", "pxo";
+      clock-output-names = "acpu_l2_aux";
+      #clock-cells = <0>;
+    };
+...
\ No newline at end of file
-- 
2.34.1

Re: [PATCH 12/16] clk: qcom: clk-krait: add 8064 errata workaround

[Dmitry Baryshkov]

On 13/03/2022 22:04, Ansuel Smith wrote:
> Add 8064 errata workaround where the sec_src clock gating needs to be

Could you please be more specific whether the errata applies only to the 
ipq8064 or to the apq8064 too? 8064 is not specific enough.

> disabled during switching. To enable this set disable_sec_src_gating in
> the mux struct.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>   drivers/clk/qcom/clk-krait.c | 16 ++++++++++++++++
>   drivers/clk/qcom/clk-krait.h |  1 +
>   drivers/clk/qcom/krait-cc.c  |  1 +
>   3 files changed, 18 insertions(+)
> 
> diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
> index d8af281eba0e..82fe7031e1f4 100644
> --- a/drivers/clk/qcom/clk-krait.c
> +++ b/drivers/clk/qcom/clk-krait.c
> @@ -18,13 +18,23 @@
>   static DEFINE_SPINLOCK(krait_clock_reg_lock);
>   
>   #define LPL_SHIFT	8
> +#define SECCLKAGD	BIT(4)
> +
>   static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
>   {
>   	unsigned long flags;
>   	u32 regval;
>   
>   	spin_lock_irqsave(&krait_clock_reg_lock, flags);
> +
>   	regval = krait_get_l2_indirect_reg(mux->offset);
> +
> +	/* 8064 Errata: disable sec_src clock gating during switch. */
> +	if (mux->disable_sec_src_gating) {
> +		regval |= SECCLKAGD;
> +		krait_set_l2_indirect_reg(mux->offset, regval);
> +	}
> +
>   	regval &= ~(mux->mask << mux->shift);
>   	regval |= (sel & mux->mask) << mux->shift;
>   	if (mux->lpl) {
> @@ -33,6 +43,12 @@ static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
>   	}
>   	krait_set_l2_indirect_reg(mux->offset, regval);
>   
> +	/* 8064 Errata: re-enabled sec_src clock gating. */

And here too

> +	if (mux->disable_sec_src_gating) {
> +		regval &= ~SECCLKAGD;
> +		krait_set_l2_indirect_reg(mux->offset, regval);
> +	}
> +
>   	/* Wait for switch to complete. */
>   	mb();
>   	udelay(1);
> diff --git a/drivers/clk/qcom/clk-krait.h b/drivers/clk/qcom/clk-krait.h
> index 9120bd2f5297..f930538c539e 100644
> --- a/drivers/clk/qcom/clk-krait.h
> +++ b/drivers/clk/qcom/clk-krait.h
> @@ -15,6 +15,7 @@ struct krait_mux_clk {
>   	u8		safe_sel;
>   	u8		old_index;
>   	bool		reparent;
> +	bool		disable_sec_src_gating;
>   
>   	struct clk_hw	hw;
>   	struct notifier_block   clk_nb;
> diff --git a/drivers/clk/qcom/krait-cc.c b/drivers/clk/qcom/krait-cc.c
> index 1bdc89c097e6..533a770332be 100644
> --- a/drivers/clk/qcom/krait-cc.c
> +++ b/drivers/clk/qcom/krait-cc.c
> @@ -154,6 +154,7 @@ krait_add_sec_mux(struct device *dev, struct clk *qsb, int id,
>   	mux->shift = 2;
>   	mux->parent_map = sec_mux_map;
>   	mux->hw.init = &init;
> +	mux->disable_sec_src_gating = true;
>   
>   	init.name = kasprintf(GFP_KERNEL, "krait%s_sec_mux", s);
>   	if (!init.name)


-- 
With best wishes
Dmitry

Re: [PATCH 12/16] clk: qcom: clk-krait: add 8064 errata workaround

[Ansuel Smith]

On Mon, Mar 14, 2022 at 11:20:21AM +0300, Dmitry Baryshkov wrote:
> On 13/03/2022 22:04, Ansuel Smith wrote:
> > Add 8064 errata workaround where the sec_src clock gating needs to be
> 
> Could you please be more specific whether the errata applies only to the
> ipq8064 or to the apq8064 too? 8064 is not specific enough.
>

That's a good question... Problem is that we really don't know the
answer. This errata comes from qsdk on an old sourcecode. I assume this
is specific to ipq8064 and apq8064 have different mux configuration.

> > disabled during switching. To enable this set disable_sec_src_gating in
> > the mux struct.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >   drivers/clk/qcom/clk-krait.c | 16 ++++++++++++++++
> >   drivers/clk/qcom/clk-krait.h |  1 +
> >   drivers/clk/qcom/krait-cc.c  |  1 +
> >   3 files changed, 18 insertions(+)
> > 
> > diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
> > index d8af281eba0e..82fe7031e1f4 100644
> > --- a/drivers/clk/qcom/clk-krait.c
> > +++ b/drivers/clk/qcom/clk-krait.c
> > @@ -18,13 +18,23 @@
> >   static DEFINE_SPINLOCK(krait_clock_reg_lock);
> >   #define LPL_SHIFT	8
> > +#define SECCLKAGD	BIT(4)
> > +
> >   static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
> >   {
> >   	unsigned long flags;
> >   	u32 regval;
> >   	spin_lock_irqsave(&krait_clock_reg_lock, flags);
> > +
> >   	regval = krait_get_l2_indirect_reg(mux->offset);
> > +
> > +	/* 8064 Errata: disable sec_src clock gating during switch. */
> > +	if (mux->disable_sec_src_gating) {
> > +		regval |= SECCLKAGD;
> > +		krait_set_l2_indirect_reg(mux->offset, regval);
> > +	}
> > +
> >   	regval &= ~(mux->mask << mux->shift);
> >   	regval |= (sel & mux->mask) << mux->shift;
> >   	if (mux->lpl) {
> > @@ -33,6 +43,12 @@ static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
> >   	}
> >   	krait_set_l2_indirect_reg(mux->offset, regval);
> > +	/* 8064 Errata: re-enabled sec_src clock gating. */
> 
> And here too
> 
> > +	if (mux->disable_sec_src_gating) {
> > +		regval &= ~SECCLKAGD;
> > +		krait_set_l2_indirect_reg(mux->offset, regval);
> > +	}
> > +
> >   	/* Wait for switch to complete. */
> >   	mb();
> >   	udelay(1);
> > diff --git a/drivers/clk/qcom/clk-krait.h b/drivers/clk/qcom/clk-krait.h
> > index 9120bd2f5297..f930538c539e 100644
> > --- a/drivers/clk/qcom/clk-krait.h
> > +++ b/drivers/clk/qcom/clk-krait.h
> > @@ -15,6 +15,7 @@ struct krait_mux_clk {
> >   	u8		safe_sel;
> >   	u8		old_index;
> >   	bool		reparent;
> > +	bool		disable_sec_src_gating;
> >   	struct clk_hw	hw;
> >   	struct notifier_block   clk_nb;
> > diff --git a/drivers/clk/qcom/krait-cc.c b/drivers/clk/qcom/krait-cc.c
> > index 1bdc89c097e6..533a770332be 100644
> > --- a/drivers/clk/qcom/krait-cc.c
> > +++ b/drivers/clk/qcom/krait-cc.c
> > @@ -154,6 +154,7 @@ krait_add_sec_mux(struct device *dev, struct clk *qsb, int id,
> >   	mux->shift = 2;
> >   	mux->parent_map = sec_mux_map;
> >   	mux->hw.init = &init;
> > +	mux->disable_sec_src_gating = true;
> >   	init.name = kasprintf(GFP_KERNEL, "krait%s_sec_mux", s);
> >   	if (!init.name)
> 
> 
> -- 
> With best wishes
> Dmitry

-- 
	Ansuel

Re: [PATCH 16/16] dt-bindings: arm: msm: Convert kpss driver Documentation to yaml

[Rob Herring]

On Sun, 13 Mar 2022 20:04:19 +0100, Ansuel Smith wrote:
> Convert kpss-acc and kpss-gcc Documentation to yaml. Fix multiple
> Documentation error and provide additional example for kpss-gcc-v2.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  .../bindings/arm/msm/qcom,kpss-acc.txt        | 49 ----------
>  .../bindings/arm/msm/qcom,kpss-acc.yaml       | 97 +++++++++++++++++++
>  .../bindings/arm/msm/qcom,kpss-gcc.txt        | 44 ---------
>  .../bindings/arm/msm/qcom,kpss-gcc.yaml       | 62 ++++++++++++
>  4 files changed, 159 insertions(+), 93 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
>  create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.yaml
>  delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
>  create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.yaml:62:4: [error] no new line character at the end of file (new-line-at-end-of-file)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1604836

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 01/16] clk: permit to define a custom parent for clk_hw_get_parent_index

[Stephen Boyd]

Quoting Ansuel Smith (2022-03-13 12:04:04)
> Clk can have multiple parents. Some clk may require to get the cached
> index of other parent that are not current associated with the clk.
> Extend clk_hw_get_parent_index() with an optional parent to permit a
> driver to get the cached index. If parent is NULL, the parent associated
> with the provided hw clk is used.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/clk/clk.c                 | 14 +++++++++-----
>  drivers/clk/tegra/clk-periph.c    |  2 +-
>  drivers/clk/tegra/clk-sdmmc-mux.c |  2 +-
>  drivers/clk/tegra/clk-super.c     |  4 ++--
>  include/linux/clk-provider.h      |  2 +-
>  5 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 8de6a22498e7..fe42f56bfbdf 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1711,15 +1711,19 @@ static int clk_fetch_parent_index(struct clk_core *core,
>  /**
>   * clk_hw_get_parent_index - return the index of the parent clock
>   * @hw: clk_hw associated with the clk being consumed
> + * @parent: optional clk_hw of the parent to be fetched
>   *
> - * Fetches and returns the index of parent clock. Returns -EINVAL if the given
> - * clock does not have a current parent.
> + * Fetches and returns the index of parent clock. If parent is not
> + * provided the parent of hw is used.
> + * Returns -EINVAL if the given clock does not have a current parent.
>   */
> -int clk_hw_get_parent_index(struct clk_hw *hw)
> +int clk_hw_get_parent_index(struct clk_hw *hw, struct clk_hw *parent)

Please introduce another API vs. tacking on an "output" argument to this
API. That makes the patch less invasive. And it can also return a
pointer instead of an integer in that case.

>  {
> -       struct clk_hw *parent = clk_hw_get_parent(hw);
> +       /* With parent NULL get the current parent of hw */
> +       if (!parent)
> +               parent = clk_hw_get_parent(hw);

Re: [PATCH 01/16] clk: permit to define a custom parent for clk_hw_get_parent_index

[Ansuel Smith]

On Tue, Mar 15, 2022 at 10:55:18AM -0700, Stephen Boyd wrote:
> Quoting Ansuel Smith (2022-03-13 12:04:04)
> > Clk can have multiple parents. Some clk may require to get the cached
> > index of other parent that are not current associated with the clk.
> > Extend clk_hw_get_parent_index() with an optional parent to permit a
> > driver to get the cached index. If parent is NULL, the parent associated
> > with the provided hw clk is used.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  drivers/clk/clk.c                 | 14 +++++++++-----
> >  drivers/clk/tegra/clk-periph.c    |  2 +-
> >  drivers/clk/tegra/clk-sdmmc-mux.c |  2 +-
> >  drivers/clk/tegra/clk-super.c     |  4 ++--
> >  include/linux/clk-provider.h      |  2 +-
> >  5 files changed, 14 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index 8de6a22498e7..fe42f56bfbdf 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -1711,15 +1711,19 @@ static int clk_fetch_parent_index(struct clk_core *core,
> >  /**
> >   * clk_hw_get_parent_index - return the index of the parent clock
> >   * @hw: clk_hw associated with the clk being consumed
> > + * @parent: optional clk_hw of the parent to be fetched
> >   *
> > - * Fetches and returns the index of parent clock. Returns -EINVAL if the given
> > - * clock does not have a current parent.
> > + * Fetches and returns the index of parent clock. If parent is not
> > + * provided the parent of hw is used.
> > + * Returns -EINVAL if the given clock does not have a current parent.
> >   */
> > -int clk_hw_get_parent_index(struct clk_hw *hw)
> > +int clk_hw_get_parent_index(struct clk_hw *hw, struct clk_hw *parent)
> 
> Please introduce another API vs. tacking on an "output" argument to this
> API. That makes the patch less invasive. And it can also return a
> pointer instead of an integer in that case.
>

Any suggestion about the name? clk_hw_fetch_parent_index? That would be
a direct access of the internal clk_fetch_parent_index.

The name is already not that intuitive as is. The alternative is to make
it extra long, don't know if that's a problem...
Something like clk_hw_get_parent_index_by_parent? (that is even more
confusing)

> >  {
> > -       struct clk_hw *parent = clk_hw_get_parent(hw);
> > +       /* With parent NULL get the current parent of hw */
> > +       if (!parent)
> > +               parent = clk_hw_get_parent(hw);

-- 
	Ansuel

Re: [PATCH 01/16] clk: permit to define a custom parent for clk_hw_get_parent_index

[Stephen Boyd]

Quoting Ansuel Smith (2022-03-15 11:07:26)
> On Tue, Mar 15, 2022 at 10:55:18AM -0700, Stephen Boyd wrote:
> > Quoting Ansuel Smith (2022-03-13 12:04:04)
> > >   */
> > > -int clk_hw_get_parent_index(struct clk_hw *hw)
> > > +int clk_hw_get_parent_index(struct clk_hw *hw, struct clk_hw *parent)
> > 
> > Please introduce another API vs. tacking on an "output" argument to this
> > API. That makes the patch less invasive. And it can also return a
> > pointer instead of an integer in that case.
> >
> 
> Any suggestion about the name? clk_hw_fetch_parent_index? That would be
> a direct access of the internal clk_fetch_parent_index.
> 
> The name is already not that intuitive as is. The alternative is to make
> it extra long, don't know if that's a problem...
> Something like clk_hw_get_parent_index_by_parent? (that is even more
> confusing)

Haha that's a mouthful. clk_hw_get_index_of_parent()? I realize now that
I misread the API because parent wasn't a const pointer. Please make
parent argument const as well and return an int as before.

Re: [PATCH 12/16] clk: qcom: clk-krait: add 8064 errata workaround

[Stephen Boyd]

Quoting Ansuel Smith (2022-03-14 05:43:20)
> On Mon, Mar 14, 2022 at 11:20:21AM +0300, Dmitry Baryshkov wrote:
> > On 13/03/2022 22:04, Ansuel Smith wrote:
> > > Add 8064 errata workaround where the sec_src clock gating needs to be
> > 
> > Could you please be more specific whether the errata applies only to the
> > ipq8064 or to the apq8064 too? 8064 is not specific enough.
> >
> 
> That's a good question... Problem is that we really don't know the
> answer. This errata comes from qsdk on an old sourcecode. I assume this
> is specific to ipq8064 and apq8064 have different mux configuration.
> 

I think it was some glitch that happened when the automatic clk gating
was enabled during a switch. The automatic clk gating didn't know that
software was running and switching the input so it killed the CPU and
stopped the clk. That lead to hangs and super badness. I assume it was
applicable to apq8064 as well because ipq8064 is basically apq8064 with
the multimedia subsystem replaced by the networking subsystem. Also I
wouldn't remember all these details because I worked on apq8064 but not
so much on ipq8064 :)

Re: [PATCH 12/16] clk: qcom: clk-krait: add 8064 errata workaround

[Ansuel Smith]

On Tue, Mar 15, 2022 at 02:34:30PM -0700, Stephen Boyd wrote:
> Quoting Ansuel Smith (2022-03-14 05:43:20)
> > On Mon, Mar 14, 2022 at 11:20:21AM +0300, Dmitry Baryshkov wrote:
> > > On 13/03/2022 22:04, Ansuel Smith wrote:
> > > > Add 8064 errata workaround where the sec_src clock gating needs to be
> > > 
> > > Could you please be more specific whether the errata applies only to the
> > > ipq8064 or to the apq8064 too? 8064 is not specific enough.
> > >
> > 
> > That's a good question... Problem is that we really don't know the
> > answer. This errata comes from qsdk on an old sourcecode. I assume this
> > is specific to ipq8064 and apq8064 have different mux configuration.
> > 
> 
> I think it was some glitch that happened when the automatic clk gating
> was enabled during a switch. The automatic clk gating didn't know that
> software was running and switching the input so it killed the CPU and
> stopped the clk. That lead to hangs and super badness. I assume it was
> applicable to apq8064 as well because ipq8064 is basically apq8064 with
> the multimedia subsystem replaced by the networking subsystem. Also I
> wouldn't remember all these details because I worked on apq8064 but not
> so much on ipq8064 :)

Honest question. Do you remember other glitch present on the platform?
We are trying to bisect an instability problem and we still needs to
find the reason. We really can't understand if it's just a power
delivery problem or a scaling problem from muxes or other things.

The current problem is that after some time the device kernel panics
with a number of strange reason like invalid kernel paging and other
strange (or the device just freze and reboots, not even a crash log)
Many kernel panics reports the crash near the mux switch (like random
error right before the mux switch) So I suspect there is a problem
there. But due to the fact that is very random we have NO exact way to
repro it. I manage sometime, while playing with the code, to repo
similar kernel crash but still i'm not sure of the real cause.

I know it's OT but do you have any idea about it? If you remember
anything about it?
(To scale the freq i'm using a dedicated cpufreq driver that works this
way:
- We first scale the cache to the max freq across all core, we set the
  voltage
- We scale the cpu to the correct target.
This is all done under a lock. Do you see anything wrong in this logic?
To mee these random crash looks to be really related to something wrong
with the mux or with the cache set to a wrong state)

Thx for any suggestion about this.
(also I will update this commit and mention both apq and ipq in the
comments)

-- 
	Ansuel

Re: [PATCH 12/16] clk: qcom: clk-krait: add 8064 errata workaround

[Stephen Boyd]

Quoting Ansuel Smith (2022-03-15 14:47:56)
> On Tue, Mar 15, 2022 at 02:34:30PM -0700, Stephen Boyd wrote:
> > Quoting Ansuel Smith (2022-03-14 05:43:20)
> > > On Mon, Mar 14, 2022 at 11:20:21AM +0300, Dmitry Baryshkov wrote:
> > > > On 13/03/2022 22:04, Ansuel Smith wrote:
> > > > > Add 8064 errata workaround where the sec_src clock gating needs to be
> > > > 
> > > > Could you please be more specific whether the errata applies only to the
> > > > ipq8064 or to the apq8064 too? 8064 is not specific enough.
> > > >
> > > 
> > > That's a good question... Problem is that we really don't know the
> > > answer. This errata comes from qsdk on an old sourcecode. I assume this
> > > is specific to ipq8064 and apq8064 have different mux configuration.
> > > 
> > 
> > I think it was some glitch that happened when the automatic clk gating
> > was enabled during a switch. The automatic clk gating didn't know that
> > software was running and switching the input so it killed the CPU and
> > stopped the clk. That lead to hangs and super badness. I assume it was
> > applicable to apq8064 as well because ipq8064 is basically apq8064 with
> > the multimedia subsystem replaced by the networking subsystem. Also I
> > wouldn't remember all these details because I worked on apq8064 but not
> > so much on ipq8064 :)
> 
> Honest question. Do you remember other glitch present on the platform?
> We are trying to bisect an instability problem and we still needs to
> find the reason. We really can't understand if it's just a power
> delivery problem or a scaling problem from muxes or other things.
> 
> The current problem is that after some time the device kernel panics
> with a number of strange reason like invalid kernel paging and other
> strange (or the device just freze and reboots, not even a crash log)
> Many kernel panics reports the crash near the mux switch (like random
> error right before the mux switch) So I suspect there is a problem
> there. But due to the fact that is very random we have NO exact way to
> repro it. I manage sometime, while playing with the code, to repo
> similar kernel crash but still i'm not sure of the real cause.
> 
> I know it's OT but do you have any idea about it? If you remember
> anything about it?
> (To scale the freq i'm using a dedicated cpufreq driver that works this
> way:
> - We first scale the cache to the max freq across all core, we set the
>   voltage
> - We scale the cpu to the correct target.
> This is all done under a lock. Do you see anything wrong in this logic?

I honestly don't remember much anymore about this. It's been a decade.
Scaling the cache used to be an independent clk and operation vs. the
CPU. Basically the clk domain and power domain for the cache was
separate from the CPU. There's also the fuse stuff that means you have
to read the fuse to know what OPP table to use. Otherwise you may be
overclocking the CPU or undervolting it. It may also be that cpuidle
can't happen during a frequency transition. Otherwise the clk gating
will be reenabled when the cpu startup code reinitializes all the cpu
registers? I'd have to look through some old vendor kernels to see if
anything jogs my memory.

> To mee these random crash looks to be really related to something wrong
> with the mux or with the cache set to a wrong state)
> 
> Thx for any suggestion about this.
> (also I will update this commit and mention both apq and ipq in the
> comments)

Re: [PATCH 07/16] clk: qcom: clk-krait: add hw_parent check for div2_round_rate

[Stephen Boyd]

Quoting Ansuel Smith (2022-03-13 12:04:10)
> Check if hw_parent is present before calculating the round_rate to
> prevent kernel panic.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/clk/qcom/clk-krait.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
> index e447fcc3806d..d8af281eba0e 100644
> --- a/drivers/clk/qcom/clk-krait.c
> +++ b/drivers/clk/qcom/clk-krait.c
> @@ -80,7 +80,12 @@ EXPORT_SYMBOL_GPL(krait_mux_clk_ops);
>  static long krait_div2_round_rate(struct clk_hw *hw, unsigned long rate,
>                                   unsigned long *parent_rate)
>  {
> -       *parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), rate * 2);
> +       struct clk_hw *hw_parent = clk_hw_get_parent(hw);
> +
> +       if (!hw_parent)
> +               return -1;

Use -EINVAL or some proper error code, not just -1

> +
> +       *parent_rate = clk_hw_round_rate(hw_parent, rate * 2);
>         return DIV_ROUND_UP(*parent_rate, 2);
>  }
>  
> -- 
> 2.34.1
>

Re: [PATCH 09/16] clk: qcom: krait-cc: drop pr_info and register qsb only if needed

[Stephen Boyd]

Quoting Ansuel Smith (2022-03-13 12:04:12)
> Drop pr_info and change them to dev_info.

Replace pr_info() with dev_info() to provide better diagnostics.

> Register qsb fixed clk only if it's not declared in DTS.
> Also reorganize variable order.

Please don't reorganize variable order.

Re: [PATCH 11/16] clk: qcom: krait-cc: force sec_mux to QSB

[Stephen Boyd]

Quoting Ansuel Smith (2022-03-13 12:04:14)
> Now that we have converted every driver to parent_data, it was
> notice that the bootloader can't really leave the system in a
> strange state where l2 or the cpu0/1 can be sourced in a number of ways
> for example cpu1 sourcing out of qsb, l2 sourcing out of pxo.
> To correctly reset the mux and the HFPLL force the sec_mux to QSB.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/clk/qcom/krait-cc.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/qcom/krait-cc.c b/drivers/clk/qcom/krait-cc.c
> index 6530f10a546f..1bdc89c097e6 100644
> --- a/drivers/clk/qcom/krait-cc.c
> +++ b/drivers/clk/qcom/krait-cc.c
> @@ -15,6 +15,8 @@
>  
>  #include "clk-krait.h"
>  
> +#define QSB_RATE       1
> +
>  static unsigned int sec_mux_map[] = {
>         2,
>         0,
> @@ -178,6 +180,12 @@ krait_add_sec_mux(struct device *dev, struct clk *qsb, int id,
>         if (ret)
>                 clk = ERR_PTR(ret);
>  
> +       /* Force the sec_mux to be set to QSB rate.

The comment start should be on a line alone

	/*
	 * Force the ...

> +        * This is needed to correctly set the parents and
> +        * to later reset mux and HFPLL to a known freq.
> +        */
> +       clk_set_rate(clk, QSB_RATE);
> +
>  err_clk:
>         if (unique_aux)
>                 kfree(parent_name);

Re: [PATCH 12/16] clk: qcom: clk-krait: add 8064 errata workaround

[Ansuel Smith]

On Tue, Mar 15, 2022 at 03:41:14PM -0700, Stephen Boyd wrote:
> Quoting Ansuel Smith (2022-03-15 14:47:56)
> > On Tue, Mar 15, 2022 at 02:34:30PM -0700, Stephen Boyd wrote:
> > > Quoting Ansuel Smith (2022-03-14 05:43:20)
> > > > On Mon, Mar 14, 2022 at 11:20:21AM +0300, Dmitry Baryshkov wrote:
> > > > > On 13/03/2022 22:04, Ansuel Smith wrote:
> > > > > > Add 8064 errata workaround where the sec_src clock gating needs to be
> > > > > 
> > > > > Could you please be more specific whether the errata applies only to the
> > > > > ipq8064 or to the apq8064 too? 8064 is not specific enough.
> > > > >
> > > > 
> > > > That's a good question... Problem is that we really don't know the
> > > > answer. This errata comes from qsdk on an old sourcecode. I assume this
> > > > is specific to ipq8064 and apq8064 have different mux configuration.
> > > > 
> > > 
> > > I think it was some glitch that happened when the automatic clk gating
> > > was enabled during a switch. The automatic clk gating didn't know that
> > > software was running and switching the input so it killed the CPU and
> > > stopped the clk. That lead to hangs and super badness. I assume it was
> > > applicable to apq8064 as well because ipq8064 is basically apq8064 with
> > > the multimedia subsystem replaced by the networking subsystem. Also I
> > > wouldn't remember all these details because I worked on apq8064 but not
> > > so much on ipq8064 :)
> > 
> > Honest question. Do you remember other glitch present on the platform?
> > We are trying to bisect an instability problem and we still needs to
> > find the reason. We really can't understand if it's just a power
> > delivery problem or a scaling problem from muxes or other things.
> > 
> > The current problem is that after some time the device kernel panics
> > with a number of strange reason like invalid kernel paging and other
> > strange (or the device just freze and reboots, not even a crash log)
> > Many kernel panics reports the crash near the mux switch (like random
> > error right before the mux switch) So I suspect there is a problem
> > there. But due to the fact that is very random we have NO exact way to
> > repro it. I manage sometime, while playing with the code, to repo
> > similar kernel crash but still i'm not sure of the real cause.
> > 
> > I know it's OT but do you have any idea about it? If you remember
> > anything about it?
> > (To scale the freq i'm using a dedicated cpufreq driver that works this
> > way:
> > - We first scale the cache to the max freq across all core, we set the
> >   voltage
> > - We scale the cpu to the correct target.
> > This is all done under a lock. Do you see anything wrong in this logic?
> 
> I honestly don't remember much anymore about this. It's been a decade.
> Scaling the cache used to be an independent clk and operation vs. the
> CPU. Basically the clk domain and power domain for the cache was
> separate from the CPU. There's also the fuse stuff that means you have
> to read the fuse to know what OPP table to use. Otherwise you may be
> overclocking the CPU or undervolting it. It may also be that cpuidle
> can't happen during a frequency transition. Otherwise the clk gating
> will be reenabled when the cpu startup code reinitializes all the cpu
> registers? I'd have to look through some old vendor kernels to see if
> anything jogs my memory.
> 
> > To mee these random crash looks to be really related to something wrong
> > with the mux or with the cache set to a wrong state)
> > 
> > Thx for any suggestion about this.
> > (also I will update this commit and mention both apq and ipq in the
> > comments)

Hi, i'm checking the spm qcom idle driver and something doesn't look
right to me... Aside from the different sequence used for boot cpu and
the abset l2 sequence, it looks like to me that WFI is enabled anyway
(even if it's not defined in the DTS or set disabled) and on top of that
it looks like we overwrite the WFI logic but we actually set to
enter power collapse (spc). Why?

Also I think we are missing the assembly code to enter wfi on krait cpu.
Am I totally confused or there are some problems in the code that nobody
notice?

-- 
	Ansuel

Re: [PATCH 12/16] clk: qcom: clk-krait: add 8064 errata workaround

[Stephen Boyd]

Quoting Ansuel Smith (2022-03-16 08:46:54)
> On Tue, Mar 15, 2022 at 03:41:14PM -0700, Stephen Boyd wrote:
> > Quoting Ansuel Smith (2022-03-15 14:47:56)
> > > On Tue, Mar 15, 2022 at 02:34:30PM -0700, Stephen Boyd wrote:
> > > > Quoting Ansuel Smith (2022-03-14 05:43:20)
> > > > > On Mon, Mar 14, 2022 at 11:20:21AM +0300, Dmitry Baryshkov wrote:
> > > > > > On 13/03/2022 22:04, Ansuel Smith wrote:
> > > > > > > Add 8064 errata workaround where the sec_src clock gating needs to be
> > > > > > 
> > > > > > Could you please be more specific whether the errata applies only to the
> > > > > > ipq8064 or to the apq8064 too? 8064 is not specific enough.
> > > > > >
> > > > > 
> > > > > That's a good question... Problem is that we really don't know the
> > > > > answer. This errata comes from qsdk on an old sourcecode. I assume this
> > > > > is specific to ipq8064 and apq8064 have different mux configuration.
> > > > > 
> > > > 
> > > > I think it was some glitch that happened when the automatic clk gating
> > > > was enabled during a switch. The automatic clk gating didn't know that
> > > > software was running and switching the input so it killed the CPU and
> > > > stopped the clk. That lead to hangs and super badness. I assume it was
> > > > applicable to apq8064 as well because ipq8064 is basically apq8064 with
> > > > the multimedia subsystem replaced by the networking subsystem. Also I
> > > > wouldn't remember all these details because I worked on apq8064 but not
> > > > so much on ipq8064 :)
> > > 
> > > Honest question. Do you remember other glitch present on the platform?
> > > We are trying to bisect an instability problem and we still needs to
> > > find the reason. We really can't understand if it's just a power
> > > delivery problem or a scaling problem from muxes or other things.
> > > 
> > > The current problem is that after some time the device kernel panics
> > > with a number of strange reason like invalid kernel paging and other
> > > strange (or the device just freze and reboots, not even a crash log)
> > > Many kernel panics reports the crash near the mux switch (like random
> > > error right before the mux switch) So I suspect there is a problem
> > > there. But due to the fact that is very random we have NO exact way to
> > > repro it. I manage sometime, while playing with the code, to repo
> > > similar kernel crash but still i'm not sure of the real cause.
> > > 
> > > I know it's OT but do you have any idea about it? If you remember
> > > anything about it?
> > > (To scale the freq i'm using a dedicated cpufreq driver that works this
> > > way:
> > > - We first scale the cache to the max freq across all core, we set the
> > >   voltage
> > > - We scale the cpu to the correct target.
> > > This is all done under a lock. Do you see anything wrong in this logic?
> > 
> > I honestly don't remember much anymore about this. It's been a decade.
> > Scaling the cache used to be an independent clk and operation vs. the
> > CPU. Basically the clk domain and power domain for the cache was
> > separate from the CPU. There's also the fuse stuff that means you have
> > to read the fuse to know what OPP table to use. Otherwise you may be
> > overclocking the CPU or undervolting it. It may also be that cpuidle
> > can't happen during a frequency transition. Otherwise the clk gating
> > will be reenabled when the cpu startup code reinitializes all the cpu
> > registers? I'd have to look through some old vendor kernels to see if
> > anything jogs my memory.
> > 
> > > To mee these random crash looks to be really related to something wrong
> > > with the mux or with the cache set to a wrong state)
> > > 
> > > Thx for any suggestion about this.
> > > (also I will update this commit and mention both apq and ipq in the
> > > comments)
> 
> Hi, i'm checking the spm qcom idle driver and something doesn't look
> right to me... Aside from the different sequence used for boot cpu and
> the abset l2 sequence, it looks like to me that WFI is enabled anyway
> (even if it's not defined in the DTS or set disabled) and on top of that
> it looks like we overwrite the WFI logic but we actually set to
> enter power collapse (spc). Why?

When the CPU is power collapsed they need to notify software running in
the secure world that the CPU is going to be reset. The CPU comes out of
reset in secure mode and it has to jump to non-secure mode. It's still a
WFI, but we don't see it in the kernel because the secure world code
executes the wfi and that runs the power collapse sequence to turn all
the power off. On power up the secure world will restore various cpu
registers (*cough* workarounds *cough*) and then switch to non-secure
mode wherever linux told it to execute at on warm boot.

> 
> Also I think we are missing the assembly code to enter wfi on krait cpu.
> Am I totally confused or there are some problems in the code that nobody
> notice?
> 

I'd expect that to run through some scm_call() path into the secure
world. The wfi can still be run by the kernel in non-secure mode, but
that will only gate the CPU clk and not actually power collapse the
core. It's a "light sleep" for the CPU. All this stuff predates PSCI but
it is very similar, just a bespoke solution instead of a standard
calling format.