[PATCH v11 0/5] PCI: qcom: Rework pipe_clk/pipe_clk_src handling

[PATCH v11 0/5] PCI: qcom: Rework pipe_clk/pipe_clk_src handling

[Dmitry Baryshkov]

PCIe pipe clk (and some other clocks) must be parked to the "safe"
source (bi_tcxo) when corresponding GDSC is turned off and on again.
Currently this is handcoded in the PCIe driver by reparenting the
gcc_pipe_N_clk_src clock.

Instead of doing it manually, follow the approach used by
clk_rcg2_shared_ops and implement this parking in the enable() and
disable() clock operations for respective pipe clocks.

Changes since v10:
 - Added linux/bitfield.h include (lkp)
 - Split fw_name/name lines in the gcc-sm8450.c (Johan)

Changes since v9:
 - Respin fixing Tested-by tags, no code changes

Changes since v8:
 - Readded .name to changed entries in gcc-sc7280 driver to restore
   compatibility with older DTS,
 - Rebased on top of linux-next, dropping reverts,
 - Verified to include all R-b tags (excuse me, Johan, I missed them
   in the previous iteration).

Changes since v7:
 - Brought back the struct clk_regmap_phy_mux (Johan)
 - Fixed includes (Stephen)
 - Dropped CLK_SET_RATE_PARENT flags from changed pipe clocks, they are
   not set in the current code and they are useless as the PHY's clock
   has fixed rate.

Changes since v6:
 - Switched the ops to use GENMASK/FIELD_GET/FIELD_PUT (Stephen),
 - As all pipe/symbol clock source clocks have the same register (and
   parents) layout, hardcode all the values. If the need arises, this
   can be changed later (Stephen),
 - Fixed commit messages and comments (suggested by Johan),
 - Added revert for the clk_regmap_mux_safe that have been already
   picked up by Bjorn.

Changes since v5:
 - Rename the clock to clk-regmap-phy-mux and the enable/disable values
   to phy_src_val and ref_src_val respectively (as recommended by
   Johan).

Changes since v4:
 - Renamed the clock to clk-regmap-pipe-src,
 - Added mention of PCIe2 PHY to the commit message,
 - Expanded commit messages to mention additional pipe clock details.

Changes since v3:
 - Replaced the clock multiplexer implementation with branch-like clock.

Changes since v2:
 - Added is_enabled() callback
 - Added default parent to the pipe clock configuration

Changes since v1:
 - Rebased on top of [1].
 - Removed erroneous Fixes tag from the patch 4.

Changes since RFC:
 - Rework clk-regmap-mux fields. Specify safe parent as P_* value rather
   than specifying the register value directly
 - Expand commit message to the first patch to specially mention that
   it is required only on newer generations of Qualcomm chipsets.

Dmitry Baryshkov (5):
  clk: qcom: regmap: add PHY clock source implementation
  clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe
    clocks
  clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe
    clocks
  PCI: qcom: Remove unnecessary pipe_clk handling
  PCI: qcom: Drop manual pipe_clk_src handling


Dmitry Baryshkov (5):
  clk: qcom: regmap: add PHY clock source implementation
  clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe
    clocks
  clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe
    clocks
  PCI: qcom: Remove unnecessary pipe_clk handling
  PCI: qcom: Drop manual pipe_clk_src handling

 drivers/clk/qcom/Makefile              |  1 +
 drivers/clk/qcom/clk-regmap-phy-mux.c  | 62 ++++++++++++++++++++
 drivers/clk/qcom/clk-regmap-phy-mux.h  | 33 +++++++++++
 drivers/clk/qcom/gcc-sc7280.c          | 49 +++++-----------
 drivers/clk/qcom/gcc-sm8450.c          | 49 +++++-----------
 drivers/pci/controller/dwc/pcie-qcom.c | 81 +-------------------------
 6 files changed, 127 insertions(+), 148 deletions(-)
 create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.c
 create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.h

-- 
2.35.1

[PATCH v11 1/5] clk: qcom: regmap: add PHY clock source implementation

[Dmitry Baryshkov]

On recent Qualcomm platforms the QMP PIPE clocks feed into a set of
muxes which must be parked to the "safe" source (bi_tcxo) when
corresponding GDSC is turned off and on again. Currently this is
handcoded in the PCIe driver by reparenting the gcc_pipe_N_clk_src
clock. However the same code sequence should be applied in the
pcie-qcom endpoint, USB3 and UFS drivers.

Rather than copying this sequence over and over again, follow the
example of clk_rcg2_shared_ops and implement this parking in the
enable() and disable() clock operations. Supplement the regmap-mux with
the new clk_regmap_phy_mux type, which implements such multiplexers
as a simple gate clocks.

This is possible since each of these multiplexers has just two clock
sources: one coming from the PHY and a reference (XO) one.  If the clock
is running off the from-PHY source, report it as enabled. Report it as
disabled otherwise (if it uses reference source).

This way the PHY will disable the pipe clock before turning off the
GDSC, which in turn would lead to disabling corresponding pipe_clk_src
(and thus it being parked to a safe, reference clock source). And vice
versa, after enabling the GDSC the PHY will enable the pipe clock, which
would cause pipe_clk_src to be switched from a safe source to the
working one.

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/clk/qcom/Makefile             |  1 +
 drivers/clk/qcom/clk-regmap-phy-mux.c | 62 +++++++++++++++++++++++++++
 drivers/clk/qcom/clk-regmap-phy-mux.h | 33 ++++++++++++++
 3 files changed, 96 insertions(+)
 create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.c
 create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.h

diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 36789f5233ef..08594230c1c1 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -11,6 +11,7 @@ clk-qcom-y += clk-branch.o
 clk-qcom-y += clk-regmap-divider.o
 clk-qcom-y += clk-regmap-mux.o
 clk-qcom-y += clk-regmap-mux-div.o
+clk-qcom-y += clk-regmap-phy-mux.o
 clk-qcom-$(CONFIG_KRAIT_CLOCKS) += clk-krait.o
 clk-qcom-y += clk-hfpll.o
 clk-qcom-y += reset.o
diff --git a/drivers/clk/qcom/clk-regmap-phy-mux.c b/drivers/clk/qcom/clk-regmap-phy-mux.c
new file mode 100644
index 000000000000..7b7243b7107d
--- /dev/null
+++ b/drivers/clk/qcom/clk-regmap-phy-mux.c
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022, Linaro Ltd.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/bitfield.h>
+#include <linux/regmap.h>
+#include <linux/export.h>
+
+#include "clk-regmap.h"
+#include "clk-regmap-phy-mux.h"
+
+#define PHY_MUX_MASK		GENMASK(1, 0)
+#define PHY_MUX_PHY_SRC		0
+#define PHY_MUX_REF_SRC		2
+
+static inline struct clk_regmap_phy_mux *to_clk_regmap_phy_mux(struct clk_regmap *clkr)
+{
+	return container_of(clkr, struct clk_regmap_phy_mux, clkr);
+}
+
+static int phy_mux_is_enabled(struct clk_hw *hw)
+{
+	struct clk_regmap *clkr = to_clk_regmap(hw);
+	struct clk_regmap_phy_mux *phy_mux = to_clk_regmap_phy_mux(clkr);
+	unsigned int val;
+
+	regmap_read(clkr->regmap, phy_mux->reg, &val);
+	val = FIELD_GET(PHY_MUX_MASK, val);
+
+	WARN_ON(val != PHY_MUX_PHY_SRC && val != PHY_MUX_REF_SRC);
+
+	return val == PHY_MUX_PHY_SRC;
+}
+
+static int phy_mux_enable(struct clk_hw *hw)
+{
+	struct clk_regmap *clkr = to_clk_regmap(hw);
+	struct clk_regmap_phy_mux *phy_mux = to_clk_regmap_phy_mux(clkr);
+
+	return regmap_update_bits(clkr->regmap, phy_mux->reg,
+				  PHY_MUX_MASK,
+				  FIELD_PREP(PHY_MUX_MASK, PHY_MUX_PHY_SRC));
+}
+
+static void phy_mux_disable(struct clk_hw *hw)
+{
+	struct clk_regmap *clkr = to_clk_regmap(hw);
+	struct clk_regmap_phy_mux *phy_mux = to_clk_regmap_phy_mux(clkr);
+
+	regmap_update_bits(clkr->regmap, phy_mux->reg,
+			   PHY_MUX_MASK,
+			   FIELD_PREP(PHY_MUX_MASK, PHY_MUX_REF_SRC));
+}
+
+const struct clk_ops clk_regmap_phy_mux_ops = {
+	.enable = phy_mux_enable,
+	.disable = phy_mux_disable,
+	.is_enabled = phy_mux_is_enabled,
+};
+EXPORT_SYMBOL_GPL(clk_regmap_phy_mux_ops);
diff --git a/drivers/clk/qcom/clk-regmap-phy-mux.h b/drivers/clk/qcom/clk-regmap-phy-mux.h
new file mode 100644
index 000000000000..614dd384695c
--- /dev/null
+++ b/drivers/clk/qcom/clk-regmap-phy-mux.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2022, Linaro Ltd.
+ */
+
+#ifndef __QCOM_CLK_REGMAP_PHY_MUX_H__
+#define __QCOM_CLK_REGMAP_PHY_MUX_H__
+
+#include "clk-regmap.h"
+
+/*
+ * A clock implementation for PHY pipe and symbols clock muxes.
+ *
+ * If the clock is running off the from-PHY source, report it as enabled.
+ * Report it as disabled otherwise (if it uses reference source).
+ *
+ * This way the PHY will disable the pipe clock before turning off the GDSC,
+ * which in turn would lead to disabling corresponding pipe_clk_src (and thus
+ * it being parked to a safe, reference clock source). And vice versa, after
+ * enabling the GDSC the PHY will enable the pipe clock, which would cause
+ * pipe_clk_src to be switched from a safe source to the working one.
+ *
+ * For some platforms this should be used for the UFS symbol_clk_src clocks
+ * too.
+ */
+struct clk_regmap_phy_mux {
+	u32			reg;
+	struct clk_regmap	clkr;
+};
+
+extern const struct clk_ops clk_regmap_phy_mux_ops;
+
+#endif
-- 
2.35.1

[PATCH v11 2/5] clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe clocks

[Dmitry Baryshkov]

Use newly defined clk_regmap_phy_mux_ops for PCIe pipe clocks to let
the clock framework automatically park the clock when the clock is
switched off and restore the parent when the clock is switched on.

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/clk/qcom/gcc-sm8450.c | 49 ++++++++++-------------------------
 1 file changed, 13 insertions(+), 36 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm8450.c b/drivers/clk/qcom/gcc-sm8450.c
index 593a195467ff..666efa5ff978 100644
--- a/drivers/clk/qcom/gcc-sm8450.c
+++ b/drivers/clk/qcom/gcc-sm8450.c
@@ -17,6 +17,7 @@
 #include "clk-regmap.h"
 #include "clk-regmap-divider.h"
 #include "clk-regmap-mux.h"
+#include "clk-regmap-phy-mux.h"
 #include "gdsc.h"
 #include "reset.h"
 
@@ -26,9 +27,7 @@ enum {
 	P_GCC_GPLL0_OUT_MAIN,
 	P_GCC_GPLL4_OUT_MAIN,
 	P_GCC_GPLL9_OUT_MAIN,
-	P_PCIE_0_PIPE_CLK,
 	P_PCIE_1_PHY_AUX_CLK,
-	P_PCIE_1_PIPE_CLK,
 	P_SLEEP_CLK,
 	P_UFS_PHY_RX_SYMBOL_0_CLK,
 	P_UFS_PHY_RX_SYMBOL_1_CLK,
@@ -153,16 +152,6 @@ static const struct clk_parent_data gcc_parent_data_3[] = {
 	{ .fw_name = "bi_tcxo" },
 };
 
-static const struct parent_map gcc_parent_map_4[] = {
-	{ P_PCIE_0_PIPE_CLK, 0 },
-	{ P_BI_TCXO, 2 },
-};
-
-static const struct clk_parent_data gcc_parent_data_4[] = {
-	{ .fw_name = "pcie_0_pipe_clk", },
-	{ .fw_name = "bi_tcxo", },
-};
-
 static const struct parent_map gcc_parent_map_5[] = {
 	{ P_PCIE_1_PHY_AUX_CLK, 0 },
 	{ P_BI_TCXO, 2 },
@@ -173,16 +162,6 @@ static const struct clk_parent_data gcc_parent_data_5[] = {
 	{ .fw_name = "bi_tcxo" },
 };
 
-static const struct parent_map gcc_parent_map_6[] = {
-	{ P_PCIE_1_PIPE_CLK, 0 },
-	{ P_BI_TCXO, 2 },
-};
-
-static const struct clk_parent_data gcc_parent_data_6[] = {
-	{ .fw_name = "pcie_1_pipe_clk" },
-	{ .fw_name = "bi_tcxo" },
-};
-
 static const struct parent_map gcc_parent_map_7[] = {
 	{ P_BI_TCXO, 0 },
 	{ P_GCC_GPLL0_OUT_MAIN, 1 },
@@ -239,17 +218,16 @@ static const struct clk_parent_data gcc_parent_data_11[] = {
 	{ .fw_name = "bi_tcxo" },
 };
 
-static struct clk_regmap_mux gcc_pcie_0_pipe_clk_src = {
+static struct clk_regmap_phy_mux gcc_pcie_0_pipe_clk_src = {
 	.reg = 0x7b060,
-	.shift = 0,
-	.width = 2,
-	.parent_map = gcc_parent_map_4,
 	.clkr = {
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_pcie_0_pipe_clk_src",
-			.parent_data = gcc_parent_data_4,
-			.num_parents = ARRAY_SIZE(gcc_parent_data_4),
-			.ops = &clk_regmap_mux_closest_ops,
+			.parent_data = &(const struct clk_parent_data){
+				.fw_name = "pcie_0_pipe_clk",
+			},
+			.num_parents = 1,
+			.ops = &clk_regmap_phy_mux_ops,
 		},
 	},
 };
@@ -269,17 +247,16 @@ static struct clk_regmap_mux gcc_pcie_1_phy_aux_clk_src = {
 	},
 };
 
-static struct clk_regmap_mux gcc_pcie_1_pipe_clk_src = {
+static struct clk_regmap_phy_mux gcc_pcie_1_pipe_clk_src = {
 	.reg = 0x9d064,
-	.shift = 0,
-	.width = 2,
-	.parent_map = gcc_parent_map_6,
 	.clkr = {
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_pcie_1_pipe_clk_src",
-			.parent_data = gcc_parent_data_6,
-			.num_parents = ARRAY_SIZE(gcc_parent_data_6),
-			.ops = &clk_regmap_mux_closest_ops,
+			.parent_data = &(const struct clk_parent_data){
+				.fw_name = "pcie_1_pipe_clk",
+			},
+			.num_parents = 1,
+			.ops = &clk_regmap_phy_mux_ops,
 		},
 	},
 };
-- 
2.35.1

[PATCH v11 3/5] clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe clocks

[Dmitry Baryshkov]

Use newly defined clk_regmap_phy_mux_ops for PCIe pipe clocks to let
the clock framework automatically park the clock when the clock is
switched off and restore the parent when the clock is switched on.

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/clk/qcom/gcc-sc7280.c | 49 +++++++++++------------------------
 1 file changed, 15 insertions(+), 34 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c
index 423627d49719..7ff64d4d5920 100644
--- a/drivers/clk/qcom/gcc-sc7280.c
+++ b/drivers/clk/qcom/gcc-sc7280.c
@@ -17,6 +17,7 @@
 #include "clk-rcg.h"
 #include "clk-regmap-divider.h"
 #include "clk-regmap-mux.h"
+#include "clk-regmap-phy-mux.h"
 #include "common.h"
 #include "gdsc.h"
 #include "reset.h"
@@ -255,26 +256,6 @@ static const struct clk_parent_data gcc_parent_data_5[] = {
 	{ .hw = &gcc_gpll0_out_even.clkr.hw },
 };
 
-static const struct parent_map gcc_parent_map_6[] = {
-	{ P_PCIE_0_PIPE_CLK, 0 },
-	{ P_BI_TCXO, 2 },
-};
-
-static const struct clk_parent_data gcc_parent_data_6[] = {
-	{ .fw_name = "pcie_0_pipe_clk", .name = "pcie_0_pipe_clk" },
-	{ .fw_name = "bi_tcxo" },
-};
-
-static const struct parent_map gcc_parent_map_7[] = {
-	{ P_PCIE_1_PIPE_CLK, 0 },
-	{ P_BI_TCXO, 2 },
-};
-
-static const struct clk_parent_data gcc_parent_data_7[] = {
-	{ .fw_name = "pcie_1_pipe_clk", .name = "pcie_1_pipe_clk" },
-	{ .fw_name = "bi_tcxo" },
-};
-
 static const struct parent_map gcc_parent_map_8[] = {
 	{ P_BI_TCXO, 0 },
 	{ P_GCC_GPLL0_OUT_MAIN, 1 },
@@ -369,32 +350,32 @@ static const struct clk_parent_data gcc_parent_data_15[] = {
 	{ .hw = &gcc_mss_gpll0_main_div_clk_src.clkr.hw },
 };
 
-static struct clk_regmap_mux gcc_pcie_0_pipe_clk_src = {
+static struct clk_regmap_phy_mux gcc_pcie_0_pipe_clk_src = {
 	.reg = 0x6b054,
-	.shift = 0,
-	.width = 2,
-	.parent_map = gcc_parent_map_6,
 	.clkr = {
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_pcie_0_pipe_clk_src",
-			.parent_data = gcc_parent_data_6,
-			.num_parents = ARRAY_SIZE(gcc_parent_data_6),
-			.ops = &clk_regmap_mux_closest_ops,
+			.parent_data = &(const struct clk_parent_data){
+				.fw_name = "pcie_0_pipe_clk",
+				.name = "pcie_0_pipe_clk",
+			},
+			.num_parents = 1,
+			.ops = &clk_regmap_phy_mux_ops,
 		},
 	},
 };
 
-static struct clk_regmap_mux gcc_pcie_1_pipe_clk_src = {
+static struct clk_regmap_phy_mux gcc_pcie_1_pipe_clk_src = {
 	.reg = 0x8d054,
-	.shift = 0,
-	.width = 2,
-	.parent_map = gcc_parent_map_7,
 	.clkr = {
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_pcie_1_pipe_clk_src",
-			.parent_data = gcc_parent_data_7,
-			.num_parents = ARRAY_SIZE(gcc_parent_data_7),
-			.ops = &clk_regmap_mux_closest_ops,
+			.parent_data = &(const struct clk_parent_data){
+				.fw_name = "pcie_1_pipe_clk",
+				.name = "pcie_1_pipe_clk",
+			},
+			.num_parents = 1,
+			.ops = &clk_regmap_phy_mux_ops,
 		},
 	},
 };
-- 
2.35.1

[PATCH v11 4/5] PCI: qcom: Remove unnecessary pipe_clk handling

[Dmitry Baryshkov]

PCIe PHY drivers (both QMP and PCIe2) already do clk_prepare_enable() /
clk_prepare_disable() pipe_clk. Remove extra calls to enable/disable
this clock from the PCIe driver, so that the PHY driver can manage the
clock on its own.

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 44 ++------------------------
 1 file changed, 3 insertions(+), 41 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 2ea13750b492..8c1073452196 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -128,7 +128,6 @@ struct qcom_pcie_resources_2_3_2 {
 	struct clk *master_clk;
 	struct clk *slave_clk;
 	struct clk *cfg_clk;
-	struct clk *pipe_clk;
 	struct regulator_bulk_data supplies[QCOM_PCIE_2_3_2_MAX_SUPPLY];
 };
 
@@ -165,7 +164,6 @@ struct qcom_pcie_resources_2_7_0 {
 	int num_clks;
 	struct regulator_bulk_data supplies[2];
 	struct reset_control *pci_reset;
-	struct clk *pipe_clk;
 	struct clk *pipe_clk_src;
 	struct clk *phy_pipe_clk;
 	struct clk *ref_clk_src;
@@ -597,8 +595,7 @@ static int qcom_pcie_get_resources_2_3_2(struct qcom_pcie *pcie)
 	if (IS_ERR(res->slave_clk))
 		return PTR_ERR(res->slave_clk);
 
-	res->pipe_clk = devm_clk_get(dev, "pipe");
-	return PTR_ERR_OR_ZERO(res->pipe_clk);
+	return 0;
 }
 
 static void qcom_pcie_deinit_2_3_2(struct qcom_pcie *pcie)
@@ -613,13 +610,6 @@ static void qcom_pcie_deinit_2_3_2(struct qcom_pcie *pcie)
 	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
 }
 
-static void qcom_pcie_post_deinit_2_3_2(struct qcom_pcie *pcie)
-{
-	struct qcom_pcie_resources_2_3_2 *res = &pcie->res.v2_3_2;
-
-	clk_disable_unprepare(res->pipe_clk);
-}
-
 static int qcom_pcie_init_2_3_2(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_resources_2_3_2 *res = &pcie->res.v2_3_2;
@@ -694,22 +684,6 @@ static int qcom_pcie_init_2_3_2(struct qcom_pcie *pcie)
 	return ret;
 }
 
-static int qcom_pcie_post_init_2_3_2(struct qcom_pcie *pcie)
-{
-	struct qcom_pcie_resources_2_3_2 *res = &pcie->res.v2_3_2;
-	struct dw_pcie *pci = pcie->pci;
-	struct device *dev = pci->dev;
-	int ret;
-
-	ret = clk_prepare_enable(res->pipe_clk);
-	if (ret) {
-		dev_err(dev, "cannot prepare/enable pipe clock\n");
-		return ret;
-	}
-
-	return 0;
-}
-
 static int qcom_pcie_get_resources_2_4_0(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_resources_2_4_0 *res = &pcie->res.v2_4_0;
@@ -1198,8 +1172,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 			return PTR_ERR(res->ref_clk_src);
 	}
 
-	res->pipe_clk = devm_clk_get(dev, "pipe");
-	return PTR_ERR_OR_ZERO(res->pipe_clk);
+	return 0;
 }
 
 static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
@@ -1292,14 +1265,7 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
 	if (pcie->cfg->pipe_clk_need_muxing)
 		clk_set_parent(res->pipe_clk_src, res->phy_pipe_clk);
 
-	return clk_prepare_enable(res->pipe_clk);
-}
-
-static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie)
-{
-	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
-
-	clk_disable_unprepare(res->pipe_clk);
+	return 0;
 }
 
 static int qcom_pcie_link_up(struct dw_pcie *pci)
@@ -1449,9 +1415,7 @@ static const struct qcom_pcie_ops ops_1_0_0 = {
 static const struct qcom_pcie_ops ops_2_3_2 = {
 	.get_resources = qcom_pcie_get_resources_2_3_2,
 	.init = qcom_pcie_init_2_3_2,
-	.post_init = qcom_pcie_post_init_2_3_2,
 	.deinit = qcom_pcie_deinit_2_3_2,
-	.post_deinit = qcom_pcie_post_deinit_2_3_2,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
 };
 
@@ -1478,7 +1442,6 @@ static const struct qcom_pcie_ops ops_2_7_0 = {
 	.deinit = qcom_pcie_deinit_2_7_0,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
 	.post_init = qcom_pcie_post_init_2_7_0,
-	.post_deinit = qcom_pcie_post_deinit_2_7_0,
 };
 
 /* Qcom IP rev.: 1.9.0 */
@@ -1488,7 +1451,6 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
 	.deinit = qcom_pcie_deinit_2_7_0,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
 	.post_init = qcom_pcie_post_init_2_7_0,
-	.post_deinit = qcom_pcie_post_deinit_2_7_0,
 	.config_sid = qcom_pcie_config_sid_sm8250,
 };
 
-- 
2.35.1

[PATCH v11 5/5] PCI: qcom: Drop manual pipe_clk_src handling

[Dmitry Baryshkov]

Manual reparenting of pipe_clk_src is being replaced with the parking of
the clock with clk_disable()/clk_enable() in the phy driver. Drop
redundant code switching of the pipe clock between the PHY clock source
and the safe bi_tcxo.

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 39 +-------------------------
 1 file changed, 1 insertion(+), 38 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 8c1073452196..9a95ecf5a688 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -164,9 +164,6 @@ struct qcom_pcie_resources_2_7_0 {
 	int num_clks;
 	struct regulator_bulk_data supplies[2];
 	struct reset_control *pci_reset;
-	struct clk *pipe_clk_src;
-	struct clk *phy_pipe_clk;
-	struct clk *ref_clk_src;
 };
 
 union qcom_pcie_resources {
@@ -192,7 +189,6 @@ struct qcom_pcie_ops {
 
 struct qcom_pcie_cfg {
 	const struct qcom_pcie_ops *ops;
-	unsigned int pipe_clk_need_muxing:1;
 	unsigned int has_tbu_clk:1;
 	unsigned int has_ddrss_sf_tbu_clk:1;
 	unsigned int has_aggre0_clk:1;
@@ -1158,20 +1154,6 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	if (ret < 0)
 		return ret;
 
-	if (pcie->cfg->pipe_clk_need_muxing) {
-		res->pipe_clk_src = devm_clk_get(dev, "pipe_mux");
-		if (IS_ERR(res->pipe_clk_src))
-			return PTR_ERR(res->pipe_clk_src);
-
-		res->phy_pipe_clk = devm_clk_get(dev, "phy_pipe");
-		if (IS_ERR(res->phy_pipe_clk))
-			return PTR_ERR(res->phy_pipe_clk);
-
-		res->ref_clk_src = devm_clk_get(dev, "ref");
-		if (IS_ERR(res->ref_clk_src))
-			return PTR_ERR(res->ref_clk_src);
-	}
-
 	return 0;
 }
 
@@ -1189,10 +1171,6 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 		return ret;
 	}
 
-	/* Set TCXO as clock source for pcie_pipe_clk_src */
-	if (pcie->cfg->pipe_clk_need_muxing)
-		clk_set_parent(res->pipe_clk_src, res->ref_clk_src);
-
 	ret = clk_bulk_prepare_enable(res->num_clks, res->clks);
 	if (ret < 0)
 		goto err_disable_regulators;
@@ -1254,18 +1232,8 @@ static void qcom_pcie_deinit_2_7_0(struct qcom_pcie *pcie)
 	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
 
 	clk_bulk_disable_unprepare(res->num_clks, res->clks);
-	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
-}
 
-static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
-{
-	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
-
-	/* Set pipe clock as clock source for pcie_pipe_clk_src */
-	if (pcie->cfg->pipe_clk_need_muxing)
-		clk_set_parent(res->pipe_clk_src, res->phy_pipe_clk);
-
-	return 0;
+	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
 }
 
 static int qcom_pcie_link_up(struct dw_pcie *pci)
@@ -1441,7 +1409,6 @@ static const struct qcom_pcie_ops ops_2_7_0 = {
 	.init = qcom_pcie_init_2_7_0,
 	.deinit = qcom_pcie_deinit_2_7_0,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
-	.post_init = qcom_pcie_post_init_2_7_0,
 };
 
 /* Qcom IP rev.: 1.9.0 */
@@ -1450,7 +1417,6 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
 	.init = qcom_pcie_init_2_7_0,
 	.deinit = qcom_pcie_deinit_2_7_0,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
-	.post_init = qcom_pcie_post_init_2_7_0,
 	.config_sid = qcom_pcie_config_sid_sm8250,
 };
 
@@ -1495,7 +1461,6 @@ static const struct qcom_pcie_cfg sm8250_cfg = {
 static const struct qcom_pcie_cfg sm8450_pcie0_cfg = {
 	.ops = &ops_1_9_0,
 	.has_ddrss_sf_tbu_clk = true,
-	.pipe_clk_need_muxing = true,
 	.has_aggre0_clk = true,
 	.has_aggre1_clk = true,
 };
@@ -1503,14 +1468,12 @@ static const struct qcom_pcie_cfg sm8450_pcie0_cfg = {
 static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
 	.ops = &ops_1_9_0,
 	.has_ddrss_sf_tbu_clk = true,
-	.pipe_clk_need_muxing = true,
 	.has_aggre1_clk = true,
 };
 
 static const struct qcom_pcie_cfg sc7280_cfg = {
 	.ops = &ops_1_9_0,
 	.has_tbu_clk = true,
-	.pipe_clk_need_muxing = true,
 };
 
 static const struct qcom_pcie_cfg sc8180x_cfg = {
-- 
2.35.1

Re: [PATCH v11 1/5] clk: qcom: regmap: add PHY clock source implementation

[Bjorn Helgaas]

On Wed, Jun 08, 2022 at 01:52:34PM +0300, Dmitry Baryshkov wrote:
> On recent Qualcomm platforms the QMP PIPE clocks feed into a set of
> muxes which must be parked to the "safe" source (bi_tcxo) when
> corresponding GDSC is turned off and on again. Currently this is
> handcoded in the PCIe driver by reparenting the gcc_pipe_N_clk_src
> clock. However the same code sequence should be applied in the
> pcie-qcom endpoint, USB3 and UFS drivers.
> 
> Rather than copying this sequence over and over again, follow the
> example of clk_rcg2_shared_ops and implement this parking in the
> enable() and disable() clock operations. Supplement the regmap-mux with
> the new clk_regmap_phy_mux type, which implements such multiplexers
> as a simple gate clocks.
> 
> This is possible since each of these multiplexers has just two clock
> sources: one coming from the PHY and a reference (XO) one.  If the clock
> is running off the from-PHY source, report it as enabled. Report it as
> disabled otherwise (if it uses reference source).
> 
> This way the PHY will disable the pipe clock before turning off the
> GDSC, which in turn would lead to disabling corresponding pipe_clk_src
> (and thus it being parked to a safe, reference clock source). And vice
> versa, after enabling the GDSC the PHY will enable the pipe clock, which
> would cause pipe_clk_src to be switched from a safe source to the
> working one.
> 
> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/clk/qcom/Makefile             |  1 +
>  drivers/clk/qcom/clk-regmap-phy-mux.c | 62 +++++++++++++++++++++++++++
>  drivers/clk/qcom/clk-regmap-phy-mux.h | 33 ++++++++++++++
>  3 files changed, 96 insertions(+)
>  create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.c
>  create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.h

Since it's posted as part of the series, I assume this should all be
applied together, so I'll look for an ack from Bjorn Andersson
<bjorn.andersson@linaro.org>, maintainer of drivers/clk/qcom.

Re: [PATCH v11 1/5] clk: qcom: regmap: add PHY clock source implementation

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-06-08 03:52:34)
> On recent Qualcomm platforms the QMP PIPE clocks feed into a set of
> muxes which must be parked to the "safe" source (bi_tcxo) when
> corresponding GDSC is turned off and on again. Currently this is
> handcoded in the PCIe driver by reparenting the gcc_pipe_N_clk_src
> clock. However the same code sequence should be applied in the
> pcie-qcom endpoint, USB3 and UFS drivers.
> 
> Rather than copying this sequence over and over again, follow the
> example of clk_rcg2_shared_ops and implement this parking in the
> enable() and disable() clock operations. Supplement the regmap-mux with
> the new clk_regmap_phy_mux type, which implements such multiplexers
> as a simple gate clocks.
> 
> This is possible since each of these multiplexers has just two clock
> sources: one coming from the PHY and a reference (XO) one.  If the clock
> is running off the from-PHY source, report it as enabled. Report it as
> disabled otherwise (if it uses reference source).
> 
> This way the PHY will disable the pipe clock before turning off the
> GDSC, which in turn would lead to disabling corresponding pipe_clk_src
> (and thus it being parked to a safe, reference clock source). And vice
> versa, after enabling the GDSC the PHY will enable the pipe clock, which
> would cause pipe_clk_src to be switched from a safe source to the
> working one.
> 
> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH v11 2/5] clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe clocks

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-06-08 03:52:35)
> Use newly defined clk_regmap_phy_mux_ops for PCIe pipe clocks to let
> the clock framework automatically park the clock when the clock is
> switched off and restore the parent when the clock is switched on.
> 
> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH v11 3/5] clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe clocks

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-06-08 03:52:36)
> Use newly defined clk_regmap_phy_mux_ops for PCIe pipe clocks to let
> the clock framework automatically park the clock when the clock is
> switched off and restore the parent when the clock is switched on.
> 
> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH v11 4/5] PCI: qcom: Remove unnecessary pipe_clk handling

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-06-08 03:52:37)
> PCIe PHY drivers (both QMP and PCIe2) already do clk_prepare_enable() /
> clk_prepare_disable() pipe_clk. Remove extra calls to enable/disable
> this clock from the PCIe driver, so that the PHY driver can manage the
> clock on its own.
> 
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH v11 5/5] PCI: qcom: Drop manual pipe_clk_src handling

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-06-08 03:52:38)
> Manual reparenting of pipe_clk_src is being replaced with the parking of
> the clock with clk_disable()/clk_enable() in the phy driver. Drop
> redundant code switching of the pipe clock between the PHY clock source
> and the safe bi_tcxo.
> 
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH v11 0/5] PCI: qcom: Rework pipe_clk/pipe_clk_src handling

[Bjorn Helgaas]

On Wed, Jun 08, 2022 at 01:52:33PM +0300, Dmitry Baryshkov wrote:
> PCIe pipe clk (and some other clocks) must be parked to the "safe"
> source (bi_tcxo) when corresponding GDSC is turned off and on again.
> Currently this is handcoded in the PCIe driver by reparenting the
> gcc_pipe_N_clk_src clock.
> 
> Instead of doing it manually, follow the approach used by
> clk_rcg2_shared_ops and implement this parking in the enable() and
> disable() clock operations for respective pipe clocks.
> 
> Changes since v10:
>  - Added linux/bitfield.h include (lkp)
>  - Split fw_name/name lines in the gcc-sm8450.c (Johan)
> 
> Changes since v9:
>  - Respin fixing Tested-by tags, no code changes
> 
> Changes since v8:
>  - Readded .name to changed entries in gcc-sc7280 driver to restore
>    compatibility with older DTS,
>  - Rebased on top of linux-next, dropping reverts,
>  - Verified to include all R-b tags (excuse me, Johan, I missed them
>    in the previous iteration).
> 
> Changes since v7:
>  - Brought back the struct clk_regmap_phy_mux (Johan)
>  - Fixed includes (Stephen)
>  - Dropped CLK_SET_RATE_PARENT flags from changed pipe clocks, they are
>    not set in the current code and they are useless as the PHY's clock
>    has fixed rate.
> 
> Changes since v6:
>  - Switched the ops to use GENMASK/FIELD_GET/FIELD_PUT (Stephen),
>  - As all pipe/symbol clock source clocks have the same register (and
>    parents) layout, hardcode all the values. If the need arises, this
>    can be changed later (Stephen),
>  - Fixed commit messages and comments (suggested by Johan),
>  - Added revert for the clk_regmap_mux_safe that have been already
>    picked up by Bjorn.
> 
> Changes since v5:
>  - Rename the clock to clk-regmap-phy-mux and the enable/disable values
>    to phy_src_val and ref_src_val respectively (as recommended by
>    Johan).
> 
> Changes since v4:
>  - Renamed the clock to clk-regmap-pipe-src,
>  - Added mention of PCIe2 PHY to the commit message,
>  - Expanded commit messages to mention additional pipe clock details.
> 
> Changes since v3:
>  - Replaced the clock multiplexer implementation with branch-like clock.
> 
> Changes since v2:
>  - Added is_enabled() callback
>  - Added default parent to the pipe clock configuration
> 
> Changes since v1:
>  - Rebased on top of [1].
>  - Removed erroneous Fixes tag from the patch 4.
> 
> Changes since RFC:
>  - Rework clk-regmap-mux fields. Specify safe parent as P_* value rather
>    than specifying the register value directly
>  - Expand commit message to the first patch to specially mention that
>    it is required only on newer generations of Qualcomm chipsets.
> 
> Dmitry Baryshkov (5):
>   clk: qcom: regmap: add PHY clock source implementation
>   clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe
>     clocks
>   clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe
>     clocks
>   PCI: qcom: Remove unnecessary pipe_clk handling
>   PCI: qcom: Drop manual pipe_clk_src handling
> 
> 
> Dmitry Baryshkov (5):
>   clk: qcom: regmap: add PHY clock source implementation
>   clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe
>     clocks
>   clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe
>     clocks
>   PCI: qcom: Remove unnecessary pipe_clk handling
>   PCI: qcom: Drop manual pipe_clk_src handling
> 
>  drivers/clk/qcom/Makefile              |  1 +
>  drivers/clk/qcom/clk-regmap-phy-mux.c  | 62 ++++++++++++++++++++
>  drivers/clk/qcom/clk-regmap-phy-mux.h  | 33 +++++++++++
>  drivers/clk/qcom/gcc-sc7280.c          | 49 +++++-----------
>  drivers/clk/qcom/gcc-sm8450.c          | 49 +++++-----------
>  drivers/pci/controller/dwc/pcie-qcom.c | 81 +-------------------------
>  6 files changed, 127 insertions(+), 148 deletions(-)
>  create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.c
>  create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.h

I applied this to pci/ctrl/qcom for v5.20, thanks!

Clock folks (Bjorn A, Andy, Michael, Stephen), I assume you're OK with
these being merged via the PCI tree.  Let me know if you prefer
anything different.

Re: [PATCH v11 1/5] clk: qcom: regmap: add PHY clock source implementation

[Bjorn Helgaas]

On Wed, Jun 08, 2022 at 01:52:34PM +0300, Dmitry Baryshkov wrote:
> On recent Qualcomm platforms the QMP PIPE clocks feed into a set of
> muxes which must be parked to the "safe" source (bi_tcxo) when
> corresponding GDSC is turned off and on again. Currently this is
> handcoded in the PCIe driver by reparenting the gcc_pipe_N_clk_src
> clock. However the same code sequence should be applied in the
> pcie-qcom endpoint, USB3 and UFS drivers.
> 
> Rather than copying this sequence over and over again, follow the
> example of clk_rcg2_shared_ops and implement this parking in the
> enable() and disable() clock operations. Supplement the regmap-mux with
> the new clk_regmap_phy_mux type, which implements such multiplexers
> as a simple gate clocks.
> 
> This is possible since each of these multiplexers has just two clock
> sources: one coming from the PHY and a reference (XO) one.  If the clock
> is running off the from-PHY source, report it as enabled. Report it as
> disabled otherwise (if it uses reference source).
> 
> This way the PHY will disable the pipe clock before turning off the
> GDSC, which in turn would lead to disabling corresponding pipe_clk_src
> (and thus it being parked to a safe, reference clock source). And vice
> versa, after enabling the GDSC the PHY will enable the pipe clock, which
> would cause pipe_clk_src to be switched from a safe source to the
> working one.
> 
> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>
> Reported-by: kernel test robot <lkp@intel.com>

FWIW, I dropped this Reported-by tag because I don't think it's really
relevant to this patch.  I think it's from this lkp report:

  https://lore.kernel.org/r/202206052344.Lkv2vI5x-lkp@intel.com

but that link wasn't included here and I don't think there's value in
including this detail about a minor build issue that was fixed before
the patch was ever applied anywhere.

Bjorn

Re: [PATCH v11 1/5] clk: qcom: regmap: add PHY clock source implementation

[Bjorn Andersson]

On Wed 08 Jun 14:22 CDT 2022, Bjorn Helgaas wrote:

> On Wed, Jun 08, 2022 at 01:52:34PM +0300, Dmitry Baryshkov wrote:
> > On recent Qualcomm platforms the QMP PIPE clocks feed into a set of
> > muxes which must be parked to the "safe" source (bi_tcxo) when
> > corresponding GDSC is turned off and on again. Currently this is
> > handcoded in the PCIe driver by reparenting the gcc_pipe_N_clk_src
> > clock. However the same code sequence should be applied in the
> > pcie-qcom endpoint, USB3 and UFS drivers.
> > 
> > Rather than copying this sequence over and over again, follow the
> > example of clk_rcg2_shared_ops and implement this parking in the
> > enable() and disable() clock operations. Supplement the regmap-mux with
> > the new clk_regmap_phy_mux type, which implements such multiplexers
> > as a simple gate clocks.
> > 
> > This is possible since each of these multiplexers has just two clock
> > sources: one coming from the PHY and a reference (XO) one.  If the clock
> > is running off the from-PHY source, report it as enabled. Report it as
> > disabled otherwise (if it uses reference source).
> > 
> > This way the PHY will disable the pipe clock before turning off the
> > GDSC, which in turn would lead to disabling corresponding pipe_clk_src
> > (and thus it being parked to a safe, reference clock source). And vice
> > versa, after enabling the GDSC the PHY will enable the pipe clock, which
> > would cause pipe_clk_src to be switched from a safe source to the
> > working one.
> > 
> > Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> > Tested-by: Johan Hovold <johan+linaro@kernel.org>
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  drivers/clk/qcom/Makefile             |  1 +
> >  drivers/clk/qcom/clk-regmap-phy-mux.c | 62 +++++++++++++++++++++++++++
> >  drivers/clk/qcom/clk-regmap-phy-mux.h | 33 ++++++++++++++
> >  3 files changed, 96 insertions(+)
> >  create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.c
> >  create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.h
> 
> Since it's posted as part of the series, I assume this should all be
> applied together, so I'll look for an ack from Bjorn Andersson
> <bjorn.andersson@linaro.org>, maintainer of drivers/clk/qcom.

Hi Bjorn,

Picking the clock patch through the clock tree would allow us to fix up
additional platforms (analog to patch 2 & 3) in time for v5.20 and
reduces risk for merge conflicts.

So please find an immutable branch (tag) of the clock patches here:
https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
tags/20220608105238.2973600-1-dmitry.baryshkov@linaro.org

Hope this suits you.

Regards,
Bjorn

Re: (subset) [PATCH v11 0/5] PCI: qcom: Rework pipe_clk/pipe_clk_src handling

[Bjorn Andersson]

On Wed, 8 Jun 2022 13:52:33 +0300, Dmitry Baryshkov wrote:
> PCIe pipe clk (and some other clocks) must be parked to the "safe"
> source (bi_tcxo) when corresponding GDSC is turned off and on again.
> Currently this is handcoded in the PCIe driver by reparenting the
> gcc_pipe_N_clk_src clock.
> 
> Instead of doing it manually, follow the approach used by
> clk_rcg2_shared_ops and implement this parking in the enable() and
> disable() clock operations for respective pipe clocks.
> 
> [...]

Applied, thanks!

[1/5] clk: qcom: regmap: add PHY clock source implementation
      commit: 74e4190cdebe5a4aa099185edb4db418fc9883e3
[2/5] clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe clocks
      commit: 7ee9d2e8b9c9f4a829cd2d77c8cba36c514f24ba
[3/5] clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe clocks
      commit: 553d12b20c10953617cc195f9e447a177c776f9d

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@linaro.org>

Re: [PATCH v11 0/5] PCI: qcom: Rework pipe_clk/pipe_clk_src handling

[Dmitry Baryshkov]

Hi,

On 16/06/2022 21:21, Bjorn Helgaas wrote:
> On Wed, Jun 08, 2022 at 01:52:33PM +0300, Dmitry Baryshkov wrote:
>> PCIe pipe clk (and some other clocks) must be parked to the "safe"
>> source (bi_tcxo) when corresponding GDSC is turned off and on again.
>> Currently this is handcoded in the PCIe driver by reparenting the
>> gcc_pipe_N_clk_src clock.
>>
>> Instead of doing it manually, follow the approach used by
>> clk_rcg2_shared_ops and implement this parking in the enable() and
>> disable() clock operations for respective pipe clocks.
>>
>> Changes since v10:
>>   - Added linux/bitfield.h include (lkp)
>>   - Split fw_name/name lines in the gcc-sm8450.c (Johan)
>>
>> Changes since v9:
>>   - Respin fixing Tested-by tags, no code changes
>>
>> Changes since v8:
>>   - Readded .name to changed entries in gcc-sc7280 driver to restore
>>     compatibility with older DTS,
>>   - Rebased on top of linux-next, dropping reverts,
>>   - Verified to include all R-b tags (excuse me, Johan, I missed them
>>     in the previous iteration).
>>
>> Changes since v7:
>>   - Brought back the struct clk_regmap_phy_mux (Johan)
>>   - Fixed includes (Stephen)
>>   - Dropped CLK_SET_RATE_PARENT flags from changed pipe clocks, they are
>>     not set in the current code and they are useless as the PHY's clock
>>     has fixed rate.
>>
>> Changes since v6:
>>   - Switched the ops to use GENMASK/FIELD_GET/FIELD_PUT (Stephen),
>>   - As all pipe/symbol clock source clocks have the same register (and
>>     parents) layout, hardcode all the values. If the need arises, this
>>     can be changed later (Stephen),
>>   - Fixed commit messages and comments (suggested by Johan),
>>   - Added revert for the clk_regmap_mux_safe that have been already
>>     picked up by Bjorn.
>>
>> Changes since v5:
>>   - Rename the clock to clk-regmap-phy-mux and the enable/disable values
>>     to phy_src_val and ref_src_val respectively (as recommended by
>>     Johan).
>>
>> Changes since v4:
>>   - Renamed the clock to clk-regmap-pipe-src,
>>   - Added mention of PCIe2 PHY to the commit message,
>>   - Expanded commit messages to mention additional pipe clock details.
>>
>> Changes since v3:
>>   - Replaced the clock multiplexer implementation with branch-like clock.
>>
>> Changes since v2:
>>   - Added is_enabled() callback
>>   - Added default parent to the pipe clock configuration
>>
>> Changes since v1:
>>   - Rebased on top of [1].
>>   - Removed erroneous Fixes tag from the patch 4.
>>
>> Changes since RFC:
>>   - Rework clk-regmap-mux fields. Specify safe parent as P_* value rather
>>     than specifying the register value directly
>>   - Expand commit message to the first patch to specially mention that
>>     it is required only on newer generations of Qualcomm chipsets.
>>
>> Dmitry Baryshkov (5):
>>    clk: qcom: regmap: add PHY clock source implementation
>>    clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe
>>      clocks
>>    clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe
>>      clocks
>>    PCI: qcom: Remove unnecessary pipe_clk handling
>>    PCI: qcom: Drop manual pipe_clk_src handling
>>
>>
>> Dmitry Baryshkov (5):
>>    clk: qcom: regmap: add PHY clock source implementation
>>    clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe
>>      clocks
>>    clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe
>>      clocks
>>    PCI: qcom: Remove unnecessary pipe_clk handling
>>    PCI: qcom: Drop manual pipe_clk_src handling
>>
>>   drivers/clk/qcom/Makefile              |  1 +
>>   drivers/clk/qcom/clk-regmap-phy-mux.c  | 62 ++++++++++++++++++++
>>   drivers/clk/qcom/clk-regmap-phy-mux.h  | 33 +++++++++++
>>   drivers/clk/qcom/gcc-sc7280.c          | 49 +++++-----------
>>   drivers/clk/qcom/gcc-sm8450.c          | 49 +++++-----------
>>   drivers/pci/controller/dwc/pcie-qcom.c | 81 +-------------------------
>>   6 files changed, 127 insertions(+), 148 deletions(-)
>>   create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.c
>>   create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.h
> 
> I applied this to pci/ctrl/qcom for v5.20, thanks!
> 
> Clock folks (Bjorn A, Andy, Michael, Stephen), I assume you're OK with
> these being merged via the PCI tree.  Let me know if you prefer
> anything different.

I noticed that this patchset is not a part of linux-next. Is it still 
pending to be merged in 5.20?

-- 
With best wishes
Dmitry

Re: [PATCH v11 0/5] PCI: qcom: Rework pipe_clk/pipe_clk_src handling

[Bjorn Helgaas]

On Thu, Jul 07, 2022 at 05:03:48PM +0300, Dmitry Baryshkov wrote:
> On 16/06/2022 21:21, Bjorn Helgaas wrote:
> > On Wed, Jun 08, 2022 at 01:52:33PM +0300, Dmitry Baryshkov wrote:
> > > PCIe pipe clk (and some other clocks) must be parked to the "safe"
> > > source (bi_tcxo) when corresponding GDSC is turned off and on again.
> > > Currently this is handcoded in the PCIe driver by reparenting the
> > > gcc_pipe_N_clk_src clock.

> > > Dmitry Baryshkov (5):
> > >    clk: qcom: regmap: add PHY clock source implementation
> > >    clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe
> > >      clocks
> > >    clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe
> > >      clocks
> > >    PCI: qcom: Remove unnecessary pipe_clk handling
> > >    PCI: qcom: Drop manual pipe_clk_src handling
> > > 
> > > 
> > > Dmitry Baryshkov (5):
> > >    clk: qcom: regmap: add PHY clock source implementation
> > >    clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe
> > >      clocks
> > >    clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe
> > >      clocks
> > >    PCI: qcom: Remove unnecessary pipe_clk handling
> > >    PCI: qcom: Drop manual pipe_clk_src handling
> > > 
> > >   drivers/clk/qcom/Makefile              |  1 +
> > >   drivers/clk/qcom/clk-regmap-phy-mux.c  | 62 ++++++++++++++++++++
> > >   drivers/clk/qcom/clk-regmap-phy-mux.h  | 33 +++++++++++
> > >   drivers/clk/qcom/gcc-sc7280.c          | 49 +++++-----------
> > >   drivers/clk/qcom/gcc-sm8450.c          | 49 +++++-----------
> > >   drivers/pci/controller/dwc/pcie-qcom.c | 81 +-------------------------
> > >   6 files changed, 127 insertions(+), 148 deletions(-)
> > >   create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.c
> > >   create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.h
> > 
> > I applied this to pci/ctrl/qcom for v5.20, thanks!
> > 
> > Clock folks (Bjorn A, Andy, Michael, Stephen), I assume you're OK with
> > these being merged via the PCI tree.  Let me know if you prefer
> > anything different.
> 
> I noticed that this patchset is not a part of linux-next. Is it still
> pending to be merged in 5.20?

It's still pending.  I currently have three separate qcom-related
branches that need to be reconciled before I put them in -next.

Bjorn

Re: [PATCH v11 0/5] PCI: qcom: Rework pipe_clk/pipe_clk_src handling

[Dmitry Baryshkov]

On Thu, 7 Jul 2022 at 18:40, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Thu, Jul 07, 2022 at 05:03:48PM +0300, Dmitry Baryshkov wrote:
> > On 16/06/2022 21:21, Bjorn Helgaas wrote:
> > > On Wed, Jun 08, 2022 at 01:52:33PM +0300, Dmitry Baryshkov wrote:
> > > > PCIe pipe clk (and some other clocks) must be parked to the "safe"
> > > > source (bi_tcxo) when corresponding GDSC is turned off and on again.
> > > > Currently this is handcoded in the PCIe driver by reparenting the
> > > > gcc_pipe_N_clk_src clock.
>
> > > > Dmitry Baryshkov (5):
> > > >    clk: qcom: regmap: add PHY clock source implementation
> > > >    clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe
> > > >      clocks
> > > >    clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe
> > > >      clocks
> > > >    PCI: qcom: Remove unnecessary pipe_clk handling
> > > >    PCI: qcom: Drop manual pipe_clk_src handling
> > > >
> > > >
> > > > Dmitry Baryshkov (5):
> > > >    clk: qcom: regmap: add PHY clock source implementation
> > > >    clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe
> > > >      clocks
> > > >    clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe
> > > >      clocks
> > > >    PCI: qcom: Remove unnecessary pipe_clk handling
> > > >    PCI: qcom: Drop manual pipe_clk_src handling
> > > >
> > > >   drivers/clk/qcom/Makefile              |  1 +
> > > >   drivers/clk/qcom/clk-regmap-phy-mux.c  | 62 ++++++++++++++++++++
> > > >   drivers/clk/qcom/clk-regmap-phy-mux.h  | 33 +++++++++++
> > > >   drivers/clk/qcom/gcc-sc7280.c          | 49 +++++-----------
> > > >   drivers/clk/qcom/gcc-sm8450.c          | 49 +++++-----------
> > > >   drivers/pci/controller/dwc/pcie-qcom.c | 81 +-------------------------
> > > >   6 files changed, 127 insertions(+), 148 deletions(-)
> > > >   create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.c
> > > >   create mode 100644 drivers/clk/qcom/clk-regmap-phy-mux.h
> > >
> > > I applied this to pci/ctrl/qcom for v5.20, thanks!
> > >
> > > Clock folks (Bjorn A, Andy, Michael, Stephen), I assume you're OK with
> > > these being merged via the PCI tree.  Let me know if you prefer
> > > anything different.
> >
> > I noticed that this patchset is not a part of linux-next. Is it still
> > pending to be merged in 5.20?
>
> It's still pending.  I currently have three separate qcom-related
> branches that need to be reconciled before I put them in -next.

Ok, thank you for the explanation. Please excuse my worries.

-- 
With best wishes
Dmitry

Re: [PATCH v11 0/5] PCI: qcom: Rework pipe_clk/pipe_clk_src handling

[Bjorn Helgaas]

[+cc Selvam, Baruch, Robert, Krishna, Krzysztof (other contributors to
qcom)]

On Thu, Jul 07, 2022 at 10:40:23AM -0500, Bjorn Helgaas wrote:
> On Thu, Jul 07, 2022 at 05:03:48PM +0300, Dmitry Baryshkov wrote:
> > On 16/06/2022 21:21, Bjorn Helgaas wrote:
> > > On Wed, Jun 08, 2022 at 01:52:33PM +0300, Dmitry Baryshkov wrote:
> > > > PCIe pipe clk (and some other clocks) must be parked to the "safe"
> > > > source (bi_tcxo) when corresponding GDSC is turned off and on again.
> > > > Currently this is handcoded in the PCIe driver by reparenting the
> > > > gcc_pipe_N_clk_src clock.

> > > > Dmitry Baryshkov (5):
> > > >    clk: qcom: regmap: add PHY clock source implementation
> > > >    clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe
> > > >      clocks
> > > >    clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe
> > > >      clocks
> > > >    PCI: qcom: Remove unnecessary pipe_clk handling
> > > >    PCI: qcom: Drop manual pipe_clk_src handling

> > > I applied this to pci/ctrl/qcom for v5.20, thanks!
> > > 
> > > Clock folks (Bjorn A, Andy, Michael, Stephen), I assume you're OK with
> > > these being merged via the PCI tree.  Let me know if you prefer
> > > anything different.
> > 
> > I noticed that this patchset is not a part of linux-next. Is it still
> > pending to be merged in 5.20?
> 
> It's still pending.  I currently have three separate qcom-related
> branches that need to be reconciled before I put them in -next.

The first three patches are on an immutable branch from the clock
tree:

  74e4190cdebe ("clk: qcom: regmap: add PHY clock source implementation")
  7ee9d2e8b9c9 ("clk: qcom: gcc-sm8450: use new clk_regmap_phy_mux_ops for PCIe pipe clocks")
  553d12b20c10 ("clk: qcom: gcc-sc7280: use new clk_regmap_phy_mux_ops for PCIe pipe clocks")

I added the rest on top of that:

  cbd27d5c2ccf ("PCI: qcom: Move IPQ8074 DBI register accesses after phy_power_on()")
  633c1fa00ab9 ("PCI: qcom: Move all DBI register accesses after phy_power_on()")
  e835e9859548 ("dt-bindings: PCI: qcom: Fix description typo")
  55e8a13ec92f ("PCI: qcom: Remove unnecessary pipe_clk handling")
  1690864ec3c8 ("PCI: dwc: tegra: move GEN3_RELATED DBI register to common header")
  39e0a12b484b ("PCI: qcom: Define slot capabilities using PCI_EXP_SLTCAP_*")
  44d07e984b93 ("PCI: qcom: Add IPQ60xx support")

and pushed it to a pci/ctrl/qcom-pending branch so you can check it
out.  It's "pending" for now because I really want an ack and some
testing for 633c1fa00ab9 ("PCI: qcom: Move all DBI register accesses
after phy_power_on()").

There's a LOT of stuff going on in qcom-land this cycle, and it's
coming from a lot of different people.  We can deal with that, but it
does complicate things and slow them down.  I think it would be easier
and speed things up if we could figure out how to coordinate things on
the qcom side.

Bjorn

Re: [PATCH v11 4/5] PCI: qcom: Remove unnecessary pipe_clk handling

[Stanimir Varbanov]

Hi Dmitry,

On 6/8/22 13:52, Dmitry Baryshkov wrote:
> PCIe PHY drivers (both QMP and PCIe2) already do clk_prepare_enable() /
> clk_prepare_disable() pipe_clk. Remove extra calls to enable/disable
> this clock from the PCIe driver, so that the PHY driver can manage the
> clock on its own.
> 
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 44 ++------------------------
>  1 file changed, 3 insertions(+), 41 deletions(-)
> 

I'm very happy to see that is gone.

Acked-by: Stanimir Varbanov <svarbanov@mm-sol.com>

-- 
regards,
Stan

Re: [PATCH v11 5/5] PCI: qcom: Drop manual pipe_clk_src handling

[Stanimir Varbanov]

Hi Dmitry,

On 6/8/22 13:52, Dmitry Baryshkov wrote:
> Manual reparenting of pipe_clk_src is being replaced with the parking of
> the clock with clk_disable()/clk_enable() in the phy driver. Drop
> redundant code switching of the pipe clock between the PHY clock source
> and the safe bi_tcxo.
> 
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 39 +-------------------------
>  1 file changed, 1 insertion(+), 38 deletions(-)
> 

Cool!

Acked-by: Stanimir Varbanov <svarbanov@mm-sol.com>


-- 
regards,
Stan

Re: [PATCH v11 5/5] PCI: qcom: Drop manual pipe_clk_src handling

[Bjorn Helgaas]

On Thu, Jul 14, 2022 at 04:08:06AM +0300, Stanimir Varbanov wrote:
> Hi Dmitry,
> 
> On 6/8/22 13:52, Dmitry Baryshkov wrote:
> > Manual reparenting of pipe_clk_src is being replaced with the parking of
> > the clock with clk_disable()/clk_enable() in the phy driver. Drop
> > redundant code switching of the pipe clock between the PHY clock source
> > and the safe bi_tcxo.
> > 
> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> > Tested-by: Johan Hovold <johan+linaro@kernel.org>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 39 +-------------------------
> >  1 file changed, 1 insertion(+), 38 deletions(-)
> 
> Cool!
> 
> Acked-by: Stanimir Varbanov <svarbanov@mm-sol.com>

Thanks, Stan.  Somehow I had applied 4/5 but not 5/5.  I added 5/5 and
your acks to both on my pci/ctrl/qcom branch for v5.20.

Bjorn