[PATCH v4 0/7] clk: qcom: Add common clkref support and migrate Glymur and Mahua

[PATCH v4 0/7] clk: qcom: Add common clkref support and migrate Glymur and Mahua

[Qiang Yu]

This series adds a common clkref_en implementation and converts glymur
and mahua to use it, along with the related binding and DTS updates.

The PCIe clkref clocks on Glymur and Mahua gate the QREF block which
provides reference clocks to the PCIe PHYs. QREF requires LDO supplies
and a reference voltage from the refgen block to operate. The refgen
block itself requires vdda-refgen_0p9 and vdda-refgen_1p2 LDOs to
function.

Previously, these QREF votes were done in PHY drivers. In earlier
discussion [1], the feedback was that this is the wrong ownership point:
those supplies are for the QREF controlled by clkref registers, not for
the PHY directly. Based on that feedback, this series keeps the
regulator handling with the clkref control path.

Another reason for this series is reuse. clkref_en registers may live in
different blocks across platforms (for example TCSR on Glymur, TLMM on
SM8750 [2]), while the behavior is the same. The common helper lets each
driver provide simple descriptors (name, offset, optional supplies) and
reuse shared registration and runtime logic.

Glymur and Mahua share the same QREF TX/RPT/RX component naming but
have different PCIe QREF topologies. Both are handled in tcsrcc-glymur.c
via match_data to select the correct descriptor table per compatible.

[1] https://lore.kernel.org/lkml/aEBfV2M-ZqDF7aRz@hovoldconsulting.com/
[2] https://lore.kernel.org/linux-arm-msm/20260202-topic-8750_tcsr-v1-0-cd7e6648c64f@oss.qualcomm.com/

Changes in v4:
- Add mahua QREF support (binding, driver, DTS) to avoid dtb check error
- Override pcie5_phy ref clock to RPMH_CXO_CLK on mahua since
  TCSR_PCIE_1_CLKREF_EN is not available
- Rename regulator arrays to topology-based names and merge duplicates
- Remove else: false blocks from binding
- Sort supply properties alphabetically in binding and DTS
- Link to v3: https://lore.kernel.org/all/20260506-qref_vote_0506-v3-0-5ab71d2e6f16@oss.qualcomm.com/

Changes in v3:
- Fix dtb check error: allOf:0: 'then' is a dependency of 'if'.
- Link to v2: https://lore.kernel.org/all/20260420-vote_qref_in_tcsrcc-v2-0-589a23ae640a@oss.qualcomm.com/

Changes in v2:
- RFC tag dropped
- Changed back to additionalProperties: false
- Moved all Glymur supply properties into top-level properties so they are explicitly defined.
- Link to v1: https://lore.kernel.org/all/20260331-qref_vote-v1-0-3fd7fbf87864@oss.qualcomm.com/

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
Qiang Yu (7):
      dt-bindings: clock: qcom,sm8550-tcsr: Add QREF/REFGEN supply properties for glymur and mahua
      clk: qcom: Add generic clkref_en support
      clk: qcom: tcsrcc-glymur: Migrate tcsr_pcie_N_clkref_en to clk_ref common helper
      clk: qcom: tcsrcc-glymur: Add Mahua QREF regulator support
      arm64: dts: qcom: glymur: Add QREF regulator supplies to TCSR
      arm64: dts: qcom: mahua: Add QREF regulator supplies to TCSR
      arm64: dts: qcom: mahua: Switch pcie5_phy ref clock to RPMH_CXO_CLK

 .../bindings/clock/qcom,sm8550-tcsr.yaml           |  66 ++++
 arch/arm64/boot/dts/qcom/glymur-crd.dts            |  20 +
 arch/arm64/boot/dts/qcom/mahua-crd.dts             |  15 +
 arch/arm64/boot/dts/qcom/mahua.dtsi                |  13 +
 drivers/clk/qcom/Makefile                          |   1 +
 drivers/clk/qcom/clk-ref.c                         | 205 +++++++++++
 drivers/clk/qcom/tcsrcc-glymur.c                   | 407 +++++++++------------
 include/linux/clk/qcom.h                           |  69 ++++
 8 files changed, 557 insertions(+), 239 deletions(-)
---
base-commit: e7e28506af98ce4e1059e5ec59334b335c00a246
change-id: 20260527-tcsr_qref_0527-f83ea225903d

Best regards,
--  
Qiang Yu <qiang.yu@oss.qualcomm.com>

[PATCH v4 1/7] dt-bindings: clock: qcom,sm8550-tcsr: Add QREF/REFGEN supply properties for glymur and mahua

[Qiang Yu]

The QREF block supplies reference clocks to PCIe PHYs and requires
dedicated LDO supplies to operate. The digital control interface for
QREF (clkref_en registers) resides in TCSR on glymur and mahua. Since
QREF has no dedicated DT node of its own, these supply properties are
placed in the TCSR node which acts as the control interface for QREF.

Document the supply properties for qcom,glymur-tcsr and qcom,mahua-tcsr.
Both SoCs share the same QREF TX/RPT/RX component naming, but differ in
topology:

Glymur has two independent QREF blocks fed by REFGEN3 and REFGEN4. Mahua
has a single QREF block fed by REFGEN3 only.

Mark the relevant supplies as required per compatible using allOf/if/then
conditionals.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 .../bindings/clock/qcom,sm8550-tcsr.yaml           | 66 ++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8550-tcsr.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8550-tcsr.yaml
index 08824f848973..fd3736ad8c8f 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8550-tcsr.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8550-tcsr.yaml
@@ -31,6 +31,7 @@ properties:
           - qcom,glymur-tcsr
           - qcom,hawi-tcsrcc
           - qcom,kaanapali-tcsr
+          - qcom,mahua-tcsr
           - qcom,milos-tcsr
           - qcom,nord-tcsrcc
           - qcom,sar2130p-tcsr
@@ -53,6 +54,71 @@ properties:
   '#reset-cells':
     const: 1
 
+  vdda-qrefrpt0-0p9-supply: true
+  vdda-qrefrpt1-0p9-supply: true
+  vdda-qrefrpt2-0p9-supply: true
+  vdda-qrefrpt3-0p9-supply: true
+  vdda-qrefrpt4-0p9-supply: true
+  vdda-qrefrpt5-0p9-supply: true
+  vdda-qrefrx0-0p9-supply: true
+  vdda-qrefrx1-0p9-supply: true
+  vdda-qrefrx2-0p9-supply: true
+  vdda-qrefrx3-0p9-supply: true
+  vdda-qrefrx4-0p9-supply: true
+  vdda-qrefrx5-0p9-supply: true
+  vdda-qreftx0-0p9-supply: true
+  vdda-qreftx0-1p2-supply: true
+  vdda-qreftx1-0p9-supply: true
+  vdda-refgen3-0p9-supply: true
+  vdda-refgen3-1p2-supply: true
+  vdda-refgen4-0p9-supply: true
+  vdda-refgen4-1p2-supply: true
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,glymur-tcsr
+    then:
+      required:
+        - vdda-qrefrpt0-0p9-supply
+        - vdda-qrefrpt1-0p9-supply
+        - vdda-qrefrpt2-0p9-supply
+        - vdda-qrefrpt3-0p9-supply
+        - vdda-qrefrpt4-0p9-supply
+        - vdda-qrefrx0-0p9-supply
+        - vdda-qrefrx1-0p9-supply
+        - vdda-qrefrx2-0p9-supply
+        - vdda-qrefrx4-0p9-supply
+        - vdda-qrefrx5-0p9-supply
+        - vdda-qreftx0-0p9-supply
+        - vdda-qreftx0-1p2-supply
+        - vdda-qreftx1-0p9-supply
+        - vdda-refgen3-0p9-supply
+        - vdda-refgen3-1p2-supply
+        - vdda-refgen4-0p9-supply
+        - vdda-refgen4-1p2-supply
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,mahua-tcsr
+    then:
+      required:
+        - vdda-qrefrpt0-0p9-supply
+        - vdda-qrefrpt1-0p9-supply
+        - vdda-qrefrpt2-0p9-supply
+        - vdda-qrefrpt3-0p9-supply
+        - vdda-qrefrpt4-0p9-supply
+        - vdda-qrefrpt5-0p9-supply
+        - vdda-qrefrx1-0p9-supply
+        - vdda-qrefrx2-0p9-supply
+        - vdda-qrefrx3-0p9-supply
+        - vdda-qreftx1-0p9-supply
+        - vdda-refgen3-0p9-supply
+        - vdda-refgen3-1p2-supply
+
 required:
   - compatible
   - clocks

-- 
2.34.1

Re: [PATCH v4 1/7] dt-bindings: clock: qcom,sm8550-tcsr: Add QREF/REFGEN supply properties for glymur and mahua

[Krzysztof Kozlowski]

On Wed, May 27, 2026 at 07:29:12PM -0700, Qiang Yu wrote:
> The QREF block supplies reference clocks to PCIe PHYs and requires
> dedicated LDO supplies to operate. The digital control interface for
> QREF (clkref_en registers) resides in TCSR on glymur and mahua. Since
> QREF has no dedicated DT node of its own, these supply properties are
> placed in the TCSR node which acts as the control interface for QREF.
> 
> Document the supply properties for qcom,glymur-tcsr and qcom,mahua-tcsr.
> Both SoCs share the same QREF TX/RPT/RX component naming, but differ in
> topology:
> 
> Glymur has two independent QREF blocks fed by REFGEN3 and REFGEN4. Mahua
> has a single QREF block fed by REFGEN3 only.
> 
> Mark the relevant supplies as required per compatible using allOf/if/then
> conditionals.

I don't think you implemented my last comments. You need own binding
file.

Best regards,
Krzysztof

Re: [PATCH v4 1/7] dt-bindings: clock: qcom,sm8550-tcsr: Add QREF/REFGEN supply properties for glymur and mahua

[Qiang Yu]

On Thu, May 28, 2026 at 09:57:10AM +0200, Krzysztof Kozlowski wrote:
> On Wed, May 27, 2026 at 07:29:12PM -0700, Qiang Yu wrote:
> > The QREF block supplies reference clocks to PCIe PHYs and requires
> > dedicated LDO supplies to operate. The digital control interface for
> > QREF (clkref_en registers) resides in TCSR on glymur and mahua. Since
> > QREF has no dedicated DT node of its own, these supply properties are
> > placed in the TCSR node which acts as the control interface for QREF.
> > 
> > Document the supply properties for qcom,glymur-tcsr and qcom,mahua-tcsr.
> > Both SoCs share the same QREF TX/RPT/RX component naming, but differ in
> > topology:
> > 
> > Glymur has two independent QREF blocks fed by REFGEN3 and REFGEN4. Mahua
> > has a single QREF block fed by REFGEN3 only.
> > 
> > Mark the relevant supplies as required per compatible using allOf/if/then
> > conditionals.
> 
> I don't think you implemented my last comments. You need own binding
> file.
>

Thanks. Do you mean qcom,glymur-tcsr and qcom,mahua-tcsr should be moved
out of qcom,sm8550-tcsr.yaml into their own binding file, e.g.
qcom,glymur-tcsr.yaml? Can I use a single file for Glymur and Mahua?

- Qiang Yu
> Best regards,
> Krzysztof
> 

Re: [PATCH v4 1/7] dt-bindings: clock: qcom,sm8550-tcsr: Add QREF/REFGEN supply properties for glymur and mahua

[Krzysztof Kozlowski]

On 28/05/2026 14:28, Qiang Yu wrote:
> On Thu, May 28, 2026 at 09:57:10AM +0200, Krzysztof Kozlowski wrote:
>> On Wed, May 27, 2026 at 07:29:12PM -0700, Qiang Yu wrote:
>>> The QREF block supplies reference clocks to PCIe PHYs and requires
>>> dedicated LDO supplies to operate. The digital control interface for
>>> QREF (clkref_en registers) resides in TCSR on glymur and mahua. Since
>>> QREF has no dedicated DT node of its own, these supply properties are
>>> placed in the TCSR node which acts as the control interface for QREF.
>>>
>>> Document the supply properties for qcom,glymur-tcsr and qcom,mahua-tcsr.
>>> Both SoCs share the same QREF TX/RPT/RX component naming, but differ in
>>> topology:
>>>
>>> Glymur has two independent QREF blocks fed by REFGEN3 and REFGEN4. Mahua
>>> has a single QREF block fed by REFGEN3 only.
>>>
>>> Mark the relevant supplies as required per compatible using allOf/if/then
>>> conditionals.
>>
>> I don't think you implemented my last comments. You need own binding
>> file.
>>
> 
> Thanks. Do you mean qcom,glymur-tcsr and qcom,mahua-tcsr should be moved
> out of qcom,sm8550-tcsr.yaml into their own binding file, e.g.

Yes.

> qcom,glymur-tcsr.yaml? Can I use a single file for Glymur and Mahua?

Single file should be fine.


Best regards,
Krzysztof

Re: [PATCH v4 1/7] dt-bindings: clock: qcom,sm8550-tcsr: Add QREF/REFGEN supply properties for glymur and mahua

[Qiang Yu]

On Thu, May 28, 2026 at 02:34:14PM +0200, Krzysztof Kozlowski wrote:
> On 28/05/2026 14:28, Qiang Yu wrote:
> > On Thu, May 28, 2026 at 09:57:10AM +0200, Krzysztof Kozlowski wrote:
> >> On Wed, May 27, 2026 at 07:29:12PM -0700, Qiang Yu wrote:
> >>> The QREF block supplies reference clocks to PCIe PHYs and requires
> >>> dedicated LDO supplies to operate. The digital control interface for
> >>> QREF (clkref_en registers) resides in TCSR on glymur and mahua. Since
> >>> QREF has no dedicated DT node of its own, these supply properties are
> >>> placed in the TCSR node which acts as the control interface for QREF.
> >>>
> >>> Document the supply properties for qcom,glymur-tcsr and qcom,mahua-tcsr.
> >>> Both SoCs share the same QREF TX/RPT/RX component naming, but differ in
> >>> topology:
> >>>
> >>> Glymur has two independent QREF blocks fed by REFGEN3 and REFGEN4. Mahua
> >>> has a single QREF block fed by REFGEN3 only.
> >>>
> >>> Mark the relevant supplies as required per compatible using allOf/if/then
> >>> conditionals.
> >>
> >> I don't think you implemented my last comments. You need own binding
> >> file.
> >>
> > 
> > Thanks. Do you mean qcom,glymur-tcsr and qcom,mahua-tcsr should be moved
> > out of qcom,sm8550-tcsr.yaml into their own binding file, e.g.
> 
> Yes.
> 
> > qcom,glymur-tcsr.yaml? Can I use a single file for Glymur and Mahua?
> 
> Single file should be fine.
>

Okay, will create a binding file for glymur and mahua in next version.

- Qiang Yu
> 
> Best regards,
> Krzysztof

[PATCH v4 2/7] clk: qcom: Add generic clkref_en support

[Qiang Yu]

Before XO refclk is distributed to PCIe/USB/eDP PHYs, it passes through
a QREF block. QREF is powered by dedicated LDO rails, and the clkref_en
register controls whether refclk is gated through to the PHY side.

These clkref controls are different from typical GCC branch clocks:
- only a single enable bit is present, without branch-style config bits
- regulators must be voted before enable and unvoted after disable

Model this as a dedicated clk_ref clock type with custom clk_ops instead
of reusing struct clk_branch semantics.

Also provide a common registration/probe API so the same clkref model
can be reused regardless of where clkref_en registers are placed, e.g.
TCSR on glymur and TLMM on SM8750.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 drivers/clk/qcom/Makefile  |   1 +
 drivers/clk/qcom/clk-ref.c | 205 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/clk/qcom.h   |  69 +++++++++++++++
 3 files changed, 275 insertions(+)

diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index e100cfd6a52d..c5b02360861d 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -8,6 +8,7 @@ clk-qcom-y += clk-pll.o
 clk-qcom-y += clk-rcg.o
 clk-qcom-y += clk-rcg2.o
 clk-qcom-y += clk-branch.o
+clk-qcom-y += clk-ref.o
 clk-qcom-y += clk-regmap-divider.o
 clk-qcom-y += clk-regmap-mux.o
 clk-qcom-y += clk-regmap-mux-div.o
diff --git a/drivers/clk/qcom/clk-ref.c b/drivers/clk/qcom/clk-ref.c
new file mode 100644
index 000000000000..213c0f58bb36
--- /dev/null
+++ b/drivers/clk/qcom/clk-ref.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2026, Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clk/qcom.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#define QCOM_CLK_REF_EN_MASK BIT(0)
+
+struct qcom_clk_ref_provider {
+	struct qcom_clk_ref *refs;
+	size_t num_refs;
+};
+
+static inline struct qcom_clk_ref *to_qcom_clk_ref(struct clk_hw *hw)
+{
+	return container_of(hw, struct qcom_clk_ref, hw);
+}
+
+static const struct clk_parent_data qcom_clk_ref_parent_data = {
+	.index = 0,
+};
+
+static int qcom_clk_ref_prepare(struct clk_hw *hw)
+{
+	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
+	int ret;
+
+	if (!rclk->desc.num_regulators)
+		return 0;
+
+	ret = regulator_bulk_enable(rclk->desc.num_regulators, rclk->regulators);
+	if (ret)
+		pr_err("Failed to enable regulators for %s: %d\n",
+		       clk_hw_get_name(hw), ret);
+
+	return ret;
+}
+
+static void qcom_clk_ref_unprepare(struct clk_hw *hw)
+{
+	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
+
+	if (rclk->desc.num_regulators)
+		regulator_bulk_disable(rclk->desc.num_regulators, rclk->regulators);
+}
+
+static int qcom_clk_ref_enable(struct clk_hw *hw)
+{
+	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
+	int ret;
+
+	ret = regmap_update_bits(rclk->regmap, rclk->desc.offset, QCOM_CLK_REF_EN_MASK,
+				 QCOM_CLK_REF_EN_MASK);
+	if (ret)
+		return ret;
+
+	udelay(10);
+
+	return 0;
+}
+
+static void qcom_clk_ref_disable(struct clk_hw *hw)
+{
+	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
+
+	regmap_update_bits(rclk->regmap, rclk->desc.offset, QCOM_CLK_REF_EN_MASK, 0);
+	udelay(10);
+}
+
+static int qcom_clk_ref_is_enabled(struct clk_hw *hw)
+{
+	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
+	u32 val;
+	int ret;
+
+	ret = regmap_read(rclk->regmap, rclk->desc.offset, &val);
+	if (ret)
+		return ret;
+
+	return !!(val & QCOM_CLK_REF_EN_MASK);
+}
+
+static const struct clk_ops qcom_clk_ref_ops = {
+	.prepare = qcom_clk_ref_prepare,
+	.unprepare = qcom_clk_ref_unprepare,
+	.enable = qcom_clk_ref_enable,
+	.disable = qcom_clk_ref_disable,
+	.is_enabled = qcom_clk_ref_is_enabled,
+};
+
+static int qcom_clk_ref_register(struct device *dev, struct regmap *regmap,
+				 struct qcom_clk_ref *clk_refs,
+				 const struct qcom_clk_ref_desc *descs,
+				 size_t num_clk_refs)
+{
+	const struct qcom_clk_ref_desc *desc;
+	struct qcom_clk_ref *clk_ref;
+	size_t clk_idx;
+	unsigned int i;
+	int ret;
+
+	for (clk_idx = 0; clk_idx < num_clk_refs; clk_idx++) {
+		clk_ref = &clk_refs[clk_idx];
+		desc = &descs[clk_idx];
+
+		if (!desc->name)
+			continue;
+
+		clk_ref->regmap = regmap;
+		clk_ref->desc = *desc;
+
+		if (clk_ref->desc.num_regulators) {
+			clk_ref->regulators = devm_kcalloc(dev, clk_ref->desc.num_regulators,
+							   sizeof(*clk_ref->regulators),
+							   GFP_KERNEL);
+			if (!clk_ref->regulators)
+				return -ENOMEM;
+
+			for (i = 0; i < clk_ref->desc.num_regulators; i++)
+				clk_ref->regulators[i].supply =
+					clk_ref->desc.regulator_names[i];
+
+			ret = devm_regulator_bulk_get(dev, clk_ref->desc.num_regulators,
+						      clk_ref->regulators);
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "Failed to get regulators for %s\n",
+						     clk_ref->desc.name);
+		}
+
+		clk_ref->init_data.name = clk_ref->desc.name;
+		clk_ref->init_data.parent_data = &qcom_clk_ref_parent_data;
+		clk_ref->init_data.num_parents = 1;
+		clk_ref->init_data.ops = &qcom_clk_ref_ops;
+		clk_ref->hw.init = &clk_ref->init_data;
+
+		ret = devm_clk_hw_register(dev, &clk_ref->hw);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static struct clk_hw *qcom_clk_ref_provider_get(struct of_phandle_args *clkspec, void *data)
+{
+	struct qcom_clk_ref_provider *provider = data;
+	unsigned int idx = clkspec->args[0];
+
+	if (idx >= provider->num_refs)
+		return ERR_PTR(-EINVAL);
+
+	if (!provider->refs[idx].regmap)
+		return ERR_PTR(-ENOENT);
+
+	return &provider->refs[idx].hw;
+}
+
+int qcom_clk_ref_probe(struct platform_device *pdev,
+		       const struct regmap_config *config,
+		       const struct qcom_clk_ref_desc *descs,
+		       size_t num_clk_refs)
+{
+	struct qcom_clk_ref_provider *provider;
+	struct device *dev = &pdev->dev;
+	struct regmap *regmap;
+	void __iomem *base;
+	int ret;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap = devm_regmap_init_mmio(dev, base, config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
+	if (!provider)
+		return -ENOMEM;
+
+	provider->refs = devm_kcalloc(dev, num_clk_refs, sizeof(*provider->refs),
+				      GFP_KERNEL);
+	if (!provider->refs)
+		return -ENOMEM;
+
+	provider->num_refs = num_clk_refs;
+
+	ret = qcom_clk_ref_register(dev, regmap, provider->refs, descs,
+				    provider->num_refs);
+	if (ret)
+		return ret;
+
+	return devm_of_clk_add_hw_provider(dev, qcom_clk_ref_provider_get, provider);
+}
+EXPORT_SYMBOL_GPL(qcom_clk_ref_probe);
diff --git a/include/linux/clk/qcom.h b/include/linux/clk/qcom.h
new file mode 100644
index 000000000000..09e2e3178cfb
--- /dev/null
+++ b/include/linux/clk/qcom.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2026, Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#ifndef __LINUX_CLK_QCOM_H
+#define __LINUX_CLK_QCOM_H
+
+#include <linux/clk-provider.h>
+#include <linux/errno.h>
+#include <linux/kconfig.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+struct device;
+struct platform_device;
+struct regulator_bulk_data;
+
+/**
+ * struct qcom_clk_ref_desc - descriptor for a clkref_en gate clock
+ * @name: clock name exposed to the common clock framework
+ * @offset: clkref_en register offset from the block base
+ * @regulator_names: optional supply names enabled while preparing the clock
+ * @num_regulators: number of entries in @regulator_names
+ */
+struct qcom_clk_ref_desc {
+	const char *name;
+	u32 offset;
+	const char * const *regulator_names;
+	unsigned int num_regulators;
+};
+
+/**
+ * struct qcom_clk_ref - per-clock data for a clkref_en gate clock
+ * @hw: common clock framework hardware clock handle
+ * @init_data: common clock framework registration data
+ * @regmap: register map backing the clkref_en register
+ * @desc: clock descriptor copied at registration time
+ * @regulators: optional bulk regulator handles for @desc.regulator_names
+ */
+struct qcom_clk_ref {
+	struct clk_hw hw;
+	struct clk_init_data init_data;
+	struct regmap *regmap;
+	struct qcom_clk_ref_desc desc;
+	struct regulator_bulk_data *regulators;
+};
+
+#if IS_ENABLED(CONFIG_COMMON_CLK_QCOM)
+
+int qcom_clk_ref_probe(struct platform_device *pdev,
+		       const struct regmap_config *config,
+		       const struct qcom_clk_ref_desc *descs,
+		       size_t num_clk_refs);
+
+#else
+
+static inline int
+qcom_clk_ref_probe(struct platform_device *pdev,
+		   const struct regmap_config *config,
+		   const struct qcom_clk_ref_desc *descs,
+		   size_t num_clk_refs)
+{
+	return -EOPNOTSUPP;
+}
+
+#endif
+
+#endif

-- 
2.34.1

Re: [PATCH v4 2/7] clk: qcom: Add generic clkref_en support

[Jie Gan]

On 5/28/2026 10:29 AM, Qiang Yu wrote:
> Before XO refclk is distributed to PCIe/USB/eDP PHYs, it passes through
> a QREF block. QREF is powered by dedicated LDO rails, and the clkref_en
> register controls whether refclk is gated through to the PHY side.
> 
> These clkref controls are different from typical GCC branch clocks:
> - only a single enable bit is present, without branch-style config bits
> - regulators must be voted before enable and unvoted after disable
> 
> Model this as a dedicated clk_ref clock type with custom clk_ops instead
> of reusing struct clk_branch semantics.
> 
> Also provide a common registration/probe API so the same clkref model
> can be reused regardless of where clkref_en registers are placed, e.g.
> TCSR on glymur and TLMM on SM8750.
> 
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> ---
>   drivers/clk/qcom/Makefile  |   1 +
>   drivers/clk/qcom/clk-ref.c | 205 +++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/clk/qcom.h   |  69 +++++++++++++++
>   3 files changed, 275 insertions(+)
> 
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index e100cfd6a52d..c5b02360861d 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -8,6 +8,7 @@ clk-qcom-y += clk-pll.o
>   clk-qcom-y += clk-rcg.o
>   clk-qcom-y += clk-rcg2.o
>   clk-qcom-y += clk-branch.o
> +clk-qcom-y += clk-ref.o
>   clk-qcom-y += clk-regmap-divider.o
>   clk-qcom-y += clk-regmap-mux.o
>   clk-qcom-y += clk-regmap-mux-div.o
> diff --git a/drivers/clk/qcom/clk-ref.c b/drivers/clk/qcom/clk-ref.c
> new file mode 100644
> index 000000000000..213c0f58bb36
> --- /dev/null
> +++ b/drivers/clk/qcom/clk-ref.c
> @@ -0,0 +1,205 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2026, Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/clk/qcom.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +
> +#define QCOM_CLK_REF_EN_MASK BIT(0)
> +
> +struct qcom_clk_ref_provider {
> +	struct qcom_clk_ref *refs;
> +	size_t num_refs;
> +};
> +
> +static inline struct qcom_clk_ref *to_qcom_clk_ref(struct clk_hw *hw)
> +{
> +	return container_of(hw, struct qcom_clk_ref, hw);
> +}
> +
> +static const struct clk_parent_data qcom_clk_ref_parent_data = {
> +	.index = 0,
> +};
> +
> +static int qcom_clk_ref_prepare(struct clk_hw *hw)
> +{
> +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> +	int ret;
> +
> +	if (!rclk->desc.num_regulators)
> +		return 0;
> +
> +	ret = regulator_bulk_enable(rclk->desc.num_regulators, rclk->regulators);
> +	if (ret)
> +		pr_err("Failed to enable regulators for %s: %d\n",
> +		       clk_hw_get_name(hw), ret);
> +
> +	return ret;
> +}
> +
> +static void qcom_clk_ref_unprepare(struct clk_hw *hw)
> +{
> +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> +
> +	if (rclk->desc.num_regulators)
> +		regulator_bulk_disable(rclk->desc.num_regulators, rclk->regulators);
> +}
> +
> +static int qcom_clk_ref_enable(struct clk_hw *hw)
> +{
> +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> +	int ret;
> +
> +	ret = regmap_update_bits(rclk->regmap, rclk->desc.offset, QCOM_CLK_REF_EN_MASK,
> +				 QCOM_CLK_REF_EN_MASK);
> +	if (ret)
> +		return ret;
> +
> +	udelay(10);

try usleep_range instead of udelay for better power consumption.

> +
> +	return 0;
> +}
> +
> +static void qcom_clk_ref_disable(struct clk_hw *hw)
> +{
> +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> +
> +	regmap_update_bits(rclk->regmap, rclk->desc.offset, QCOM_CLK_REF_EN_MASK, 0);
> +	udelay(10);
> +}
> +
> +static int qcom_clk_ref_is_enabled(struct clk_hw *hw)
> +{
> +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read(rclk->regmap, rclk->desc.offset, &val);
> +	if (ret)
> +		return ret;

regmap_read returns a negative error code on failure, but the 
clk_ops.is_enabled() treats the non-zero value as enabled.

Thanks,
Jie

> +
> +	return !!(val & QCOM_CLK_REF_EN_MASK);
> +}
> +
> +static const struct clk_ops qcom_clk_ref_ops = {
> +	.prepare = qcom_clk_ref_prepare,
> +	.unprepare = qcom_clk_ref_unprepare,
> +	.enable = qcom_clk_ref_enable,
> +	.disable = qcom_clk_ref_disable,
> +	.is_enabled = qcom_clk_ref_is_enabled,
> +};
> +
> +static int qcom_clk_ref_register(struct device *dev, struct regmap *regmap,
> +				 struct qcom_clk_ref *clk_refs,
> +				 const struct qcom_clk_ref_desc *descs,
> +				 size_t num_clk_refs)
> +{
> +	const struct qcom_clk_ref_desc *desc;
> +	struct qcom_clk_ref *clk_ref;
> +	size_t clk_idx;
> +	unsigned int i;
> +	int ret;
> +
> +	for (clk_idx = 0; clk_idx < num_clk_refs; clk_idx++) {
> +		clk_ref = &clk_refs[clk_idx];
> +		desc = &descs[clk_idx];
> +
> +		if (!desc->name)
> +			continue;
> +
> +		clk_ref->regmap = regmap;
> +		clk_ref->desc = *desc;
> +
> +		if (clk_ref->desc.num_regulators) {
> +			clk_ref->regulators = devm_kcalloc(dev, clk_ref->desc.num_regulators,
> +							   sizeof(*clk_ref->regulators),
> +							   GFP_KERNEL);
> +			if (!clk_ref->regulators)
> +				return -ENOMEM;
> +
> +			for (i = 0; i < clk_ref->desc.num_regulators; i++)
> +				clk_ref->regulators[i].supply =
> +					clk_ref->desc.regulator_names[i];
> +
> +			ret = devm_regulator_bulk_get(dev, clk_ref->desc.num_regulators,
> +						      clk_ref->regulators);
> +			if (ret)
> +				return dev_err_probe(dev, ret,
> +						     "Failed to get regulators for %s\n",
> +						     clk_ref->desc.name);
> +		}
> +
> +		clk_ref->init_data.name = clk_ref->desc.name;
> +		clk_ref->init_data.parent_data = &qcom_clk_ref_parent_data;
> +		clk_ref->init_data.num_parents = 1;
> +		clk_ref->init_data.ops = &qcom_clk_ref_ops;
> +		clk_ref->hw.init = &clk_ref->init_data;
> +
> +		ret = devm_clk_hw_register(dev, &clk_ref->hw);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct clk_hw *qcom_clk_ref_provider_get(struct of_phandle_args *clkspec, void *data)
> +{
> +	struct qcom_clk_ref_provider *provider = data;
> +	unsigned int idx = clkspec->args[0];
> +
> +	if (idx >= provider->num_refs)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (!provider->refs[idx].regmap)
> +		return ERR_PTR(-ENOENT);
> +
> +	return &provider->refs[idx].hw;
> +}
> +
> +int qcom_clk_ref_probe(struct platform_device *pdev,
> +		       const struct regmap_config *config,
> +		       const struct qcom_clk_ref_desc *descs,
> +		       size_t num_clk_refs)
> +{
> +	struct qcom_clk_ref_provider *provider;
> +	struct device *dev = &pdev->dev;
> +	struct regmap *regmap;
> +	void __iomem *base;
> +	int ret;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	regmap = devm_regmap_init_mmio(dev, base, config);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
> +	if (!provider)
> +		return -ENOMEM;
> +
> +	provider->refs = devm_kcalloc(dev, num_clk_refs, sizeof(*provider->refs),
> +				      GFP_KERNEL);
> +	if (!provider->refs)
> +		return -ENOMEM;
> +
> +	provider->num_refs = num_clk_refs;
> +
> +	ret = qcom_clk_ref_register(dev, regmap, provider->refs, descs,
> +				    provider->num_refs);
> +	if (ret)
> +		return ret;
> +
> +	return devm_of_clk_add_hw_provider(dev, qcom_clk_ref_provider_get, provider);
> +}
> +EXPORT_SYMBOL_GPL(qcom_clk_ref_probe);
> diff --git a/include/linux/clk/qcom.h b/include/linux/clk/qcom.h
> new file mode 100644
> index 000000000000..09e2e3178cfb
> --- /dev/null
> +++ b/include/linux/clk/qcom.h
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2026, Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#ifndef __LINUX_CLK_QCOM_H
> +#define __LINUX_CLK_QCOM_H
> +
> +#include <linux/clk-provider.h>
> +#include <linux/errno.h>
> +#include <linux/kconfig.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +
> +struct device;
> +struct platform_device;
> +struct regulator_bulk_data;
> +
> +/**
> + * struct qcom_clk_ref_desc - descriptor for a clkref_en gate clock
> + * @name: clock name exposed to the common clock framework
> + * @offset: clkref_en register offset from the block base
> + * @regulator_names: optional supply names enabled while preparing the clock
> + * @num_regulators: number of entries in @regulator_names
> + */
> +struct qcom_clk_ref_desc {
> +	const char *name;
> +	u32 offset;
> +	const char * const *regulator_names;
> +	unsigned int num_regulators;
> +};
> +
> +/**
> + * struct qcom_clk_ref - per-clock data for a clkref_en gate clock
> + * @hw: common clock framework hardware clock handle
> + * @init_data: common clock framework registration data
> + * @regmap: register map backing the clkref_en register
> + * @desc: clock descriptor copied at registration time
> + * @regulators: optional bulk regulator handles for @desc.regulator_names
> + */
> +struct qcom_clk_ref {
> +	struct clk_hw hw;
> +	struct clk_init_data init_data;
> +	struct regmap *regmap;
> +	struct qcom_clk_ref_desc desc;
> +	struct regulator_bulk_data *regulators;
> +};
> +
> +#if IS_ENABLED(CONFIG_COMMON_CLK_QCOM)
> +
> +int qcom_clk_ref_probe(struct platform_device *pdev,
> +		       const struct regmap_config *config,
> +		       const struct qcom_clk_ref_desc *descs,
> +		       size_t num_clk_refs);
> +
> +#else
> +
> +static inline int
> +qcom_clk_ref_probe(struct platform_device *pdev,
> +		   const struct regmap_config *config,
> +		   const struct qcom_clk_ref_desc *descs,
> +		   size_t num_clk_refs)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +#endif
> +
> +#endif
> 

Re: [PATCH v4 2/7] clk: qcom: Add generic clkref_en support

[Qiang Yu]

On Thu, May 28, 2026 at 11:03:45AM +0800, Jie Gan wrote:
> 
> 
> On 5/28/2026 10:29 AM, Qiang Yu wrote:
> > Before XO refclk is distributed to PCIe/USB/eDP PHYs, it passes through
> > a QREF block. QREF is powered by dedicated LDO rails, and the clkref_en
> > register controls whether refclk is gated through to the PHY side.
> > 
> > These clkref controls are different from typical GCC branch clocks:
> > - only a single enable bit is present, without branch-style config bits
> > - regulators must be voted before enable and unvoted after disable
> > 
> > Model this as a dedicated clk_ref clock type with custom clk_ops instead
> > of reusing struct clk_branch semantics.
> > 
> > Also provide a common registration/probe API so the same clkref model
> > can be reused regardless of where clkref_en registers are placed, e.g.
> > TCSR on glymur and TLMM on SM8750.
> > 
> > Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> > ---
> >   drivers/clk/qcom/Makefile  |   1 +
> >   drivers/clk/qcom/clk-ref.c | 205 +++++++++++++++++++++++++++++++++++++++++++++
> >   include/linux/clk/qcom.h   |  69 +++++++++++++++
> >   3 files changed, 275 insertions(+)
> > 
> > diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> > index e100cfd6a52d..c5b02360861d 100644
> > --- a/drivers/clk/qcom/Makefile
> > +++ b/drivers/clk/qcom/Makefile
> > @@ -8,6 +8,7 @@ clk-qcom-y += clk-pll.o
> >   clk-qcom-y += clk-rcg.o
> >   clk-qcom-y += clk-rcg2.o
> >   clk-qcom-y += clk-branch.o
> > +clk-qcom-y += clk-ref.o
> >   clk-qcom-y += clk-regmap-divider.o
> >   clk-qcom-y += clk-regmap-mux.o
> >   clk-qcom-y += clk-regmap-mux-div.o
> > diff --git a/drivers/clk/qcom/clk-ref.c b/drivers/clk/qcom/clk-ref.c
> > new file mode 100644
> > index 000000000000..213c0f58bb36
> > --- /dev/null
> > +++ b/drivers/clk/qcom/clk-ref.c
> > @@ -0,0 +1,205 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2026, Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/clk/qcom.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/export.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/slab.h>
> > +
> > +#define QCOM_CLK_REF_EN_MASK BIT(0)
> > +
> > +struct qcom_clk_ref_provider {
> > +	struct qcom_clk_ref *refs;
> > +	size_t num_refs;
> > +};
> > +
> > +static inline struct qcom_clk_ref *to_qcom_clk_ref(struct clk_hw *hw)
> > +{
> > +	return container_of(hw, struct qcom_clk_ref, hw);
> > +}
> > +
> > +static const struct clk_parent_data qcom_clk_ref_parent_data = {
> > +	.index = 0,
> > +};
> > +
> > +static int qcom_clk_ref_prepare(struct clk_hw *hw)
> > +{
> > +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> > +	int ret;
> > +
> > +	if (!rclk->desc.num_regulators)
> > +		return 0;
> > +
> > +	ret = regulator_bulk_enable(rclk->desc.num_regulators, rclk->regulators);
> > +	if (ret)
> > +		pr_err("Failed to enable regulators for %s: %d\n",
> > +		       clk_hw_get_name(hw), ret);
> > +
> > +	return ret;
> > +}
> > +
> > +static void qcom_clk_ref_unprepare(struct clk_hw *hw)
> > +{
> > +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> > +
> > +	if (rclk->desc.num_regulators)
> > +		regulator_bulk_disable(rclk->desc.num_regulators, rclk->regulators);
> > +}
> > +
> > +static int qcom_clk_ref_enable(struct clk_hw *hw)
> > +{
> > +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> > +	int ret;
> > +
> > +	ret = regmap_update_bits(rclk->regmap, rclk->desc.offset, QCOM_CLK_REF_EN_MASK,
> > +				 QCOM_CLK_REF_EN_MASK);
> > +	if (ret)
> > +		return ret;
> > +
> > +	udelay(10);
> 
> try usleep_range instead of udelay for better power consumption.
>

The .enable and .disable callbacks are called under the clock framework's
enable spinlock, so sleeping is not allowed here. udelay is intentional.

> > +
> > +	return 0;
> > +}
> > +
> > +static void qcom_clk_ref_disable(struct clk_hw *hw)
> > +{
> > +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> > +
> > +	regmap_update_bits(rclk->regmap, rclk->desc.offset, QCOM_CLK_REF_EN_MASK, 0);
> > +	udelay(10);
> > +}
> > +
> > +static int qcom_clk_ref_is_enabled(struct clk_hw *hw)
> > +{
> > +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> > +	u32 val;
> > +	int ret;
> > +
> > +	ret = regmap_read(rclk->regmap, rclk->desc.offset, &val);
> > +	if (ret)
> > +		return ret;
> 
> regmap_read returns a negative error code on failure, but the
> clk_ops.is_enabled() treats the non-zero value as enabled.
>

A regmap_read failure doesn't mean the clock is disabled.

- Qiang Yu
> Thanks,
> Jie
> 
> > +
> > +	return !!(val & QCOM_CLK_REF_EN_MASK);
> > +}
> > +
> > +static const struct clk_ops qcom_clk_ref_ops = {
> > +	.prepare = qcom_clk_ref_prepare,
> > +	.unprepare = qcom_clk_ref_unprepare,
> > +	.enable = qcom_clk_ref_enable,
> > +	.disable = qcom_clk_ref_disable,
> > +	.is_enabled = qcom_clk_ref_is_enabled,
> > +};
> > +
> > +static int qcom_clk_ref_register(struct device *dev, struct regmap *regmap,
> > +				 struct qcom_clk_ref *clk_refs,
> > +				 const struct qcom_clk_ref_desc *descs,
> > +				 size_t num_clk_refs)
> > +{
> > +	const struct qcom_clk_ref_desc *desc;
> > +	struct qcom_clk_ref *clk_ref;
> > +	size_t clk_idx;
> > +	unsigned int i;
> > +	int ret;
> > +
> > +	for (clk_idx = 0; clk_idx < num_clk_refs; clk_idx++) {
> > +		clk_ref = &clk_refs[clk_idx];
> > +		desc = &descs[clk_idx];
> > +
> > +		if (!desc->name)
> > +			continue;
> > +
> > +		clk_ref->regmap = regmap;
> > +		clk_ref->desc = *desc;
> > +
> > +		if (clk_ref->desc.num_regulators) {
> > +			clk_ref->regulators = devm_kcalloc(dev, clk_ref->desc.num_regulators,
> > +							   sizeof(*clk_ref->regulators),
> > +							   GFP_KERNEL);
> > +			if (!clk_ref->regulators)
> > +				return -ENOMEM;
> > +
> > +			for (i = 0; i < clk_ref->desc.num_regulators; i++)
> > +				clk_ref->regulators[i].supply =
> > +					clk_ref->desc.regulator_names[i];
> > +
> > +			ret = devm_regulator_bulk_get(dev, clk_ref->desc.num_regulators,
> > +						      clk_ref->regulators);
> > +			if (ret)
> > +				return dev_err_probe(dev, ret,
> > +						     "Failed to get regulators for %s\n",
> > +						     clk_ref->desc.name);
> > +		}
> > +
> > +		clk_ref->init_data.name = clk_ref->desc.name;
> > +		clk_ref->init_data.parent_data = &qcom_clk_ref_parent_data;
> > +		clk_ref->init_data.num_parents = 1;
> > +		clk_ref->init_data.ops = &qcom_clk_ref_ops;
> > +		clk_ref->hw.init = &clk_ref->init_data;
> > +
> > +		ret = devm_clk_hw_register(dev, &clk_ref->hw);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static struct clk_hw *qcom_clk_ref_provider_get(struct of_phandle_args *clkspec, void *data)
> > +{
> > +	struct qcom_clk_ref_provider *provider = data;
> > +	unsigned int idx = clkspec->args[0];
> > +
> > +	if (idx >= provider->num_refs)
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	if (!provider->refs[idx].regmap)
> > +		return ERR_PTR(-ENOENT);
> > +
> > +	return &provider->refs[idx].hw;
> > +}
> > +
> > +int qcom_clk_ref_probe(struct platform_device *pdev,
> > +		       const struct regmap_config *config,
> > +		       const struct qcom_clk_ref_desc *descs,
> > +		       size_t num_clk_refs)
> > +{
> > +	struct qcom_clk_ref_provider *provider;
> > +	struct device *dev = &pdev->dev;
> > +	struct regmap *regmap;
> > +	void __iomem *base;
> > +	int ret;
> > +
> > +	base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(base))
> > +		return PTR_ERR(base);
> > +
> > +	regmap = devm_regmap_init_mmio(dev, base, config);
> > +	if (IS_ERR(regmap))
> > +		return PTR_ERR(regmap);
> > +
> > +	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
> > +	if (!provider)
> > +		return -ENOMEM;
> > +
> > +	provider->refs = devm_kcalloc(dev, num_clk_refs, sizeof(*provider->refs),
> > +				      GFP_KERNEL);
> > +	if (!provider->refs)
> > +		return -ENOMEM;
> > +
> > +	provider->num_refs = num_clk_refs;
> > +
> > +	ret = qcom_clk_ref_register(dev, regmap, provider->refs, descs,
> > +				    provider->num_refs);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return devm_of_clk_add_hw_provider(dev, qcom_clk_ref_provider_get, provider);
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_clk_ref_probe);
> > diff --git a/include/linux/clk/qcom.h b/include/linux/clk/qcom.h
> > new file mode 100644
> > index 000000000000..09e2e3178cfb
> > --- /dev/null
> > +++ b/include/linux/clk/qcom.h
> > @@ -0,0 +1,69 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2026, Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + */
> > +
> > +#ifndef __LINUX_CLK_QCOM_H
> > +#define __LINUX_CLK_QCOM_H
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/errno.h>
> > +#include <linux/kconfig.h>
> > +#include <linux/regmap.h>
> > +#include <linux/types.h>
> > +
> > +struct device;
> > +struct platform_device;
> > +struct regulator_bulk_data;
> > +
> > +/**
> > + * struct qcom_clk_ref_desc - descriptor for a clkref_en gate clock
> > + * @name: clock name exposed to the common clock framework
> > + * @offset: clkref_en register offset from the block base
> > + * @regulator_names: optional supply names enabled while preparing the clock
> > + * @num_regulators: number of entries in @regulator_names
> > + */
> > +struct qcom_clk_ref_desc {
> > +	const char *name;
> > +	u32 offset;
> > +	const char * const *regulator_names;
> > +	unsigned int num_regulators;
> > +};
> > +
> > +/**
> > + * struct qcom_clk_ref - per-clock data for a clkref_en gate clock
> > + * @hw: common clock framework hardware clock handle
> > + * @init_data: common clock framework registration data
> > + * @regmap: register map backing the clkref_en register
> > + * @desc: clock descriptor copied at registration time
> > + * @regulators: optional bulk regulator handles for @desc.regulator_names
> > + */
> > +struct qcom_clk_ref {
> > +	struct clk_hw hw;
> > +	struct clk_init_data init_data;
> > +	struct regmap *regmap;
> > +	struct qcom_clk_ref_desc desc;
> > +	struct regulator_bulk_data *regulators;
> > +};
> > +
> > +#if IS_ENABLED(CONFIG_COMMON_CLK_QCOM)
> > +
> > +int qcom_clk_ref_probe(struct platform_device *pdev,
> > +		       const struct regmap_config *config,
> > +		       const struct qcom_clk_ref_desc *descs,
> > +		       size_t num_clk_refs);
> > +
> > +#else
> > +
> > +static inline int
> > +qcom_clk_ref_probe(struct platform_device *pdev,
> > +		   const struct regmap_config *config,
> > +		   const struct qcom_clk_ref_desc *descs,
> > +		   size_t num_clk_refs)
> > +{
> > +	return -EOPNOTSUPP;
> > +}
> > +
> > +#endif
> > +
> > +#endif
> > 
> 

Re: [PATCH v4 2/7] clk: qcom: Add generic clkref_en support

[Jie Gan]

On 5/28/2026 9:06 PM, Qiang Yu wrote:
> On Thu, May 28, 2026 at 11:03:45AM +0800, Jie Gan wrote:
>>
>>
>> On 5/28/2026 10:29 AM, Qiang Yu wrote:
>>> Before XO refclk is distributed to PCIe/USB/eDP PHYs, it passes through
>>> a QREF block. QREF is powered by dedicated LDO rails, and the clkref_en
>>> register controls whether refclk is gated through to the PHY side.
>>>
>>> These clkref controls are different from typical GCC branch clocks:
>>> - only a single enable bit is present, without branch-style config bits
>>> - regulators must be voted before enable and unvoted after disable
>>>
>>> Model this as a dedicated clk_ref clock type with custom clk_ops instead
>>> of reusing struct clk_branch semantics.
>>>
>>> Also provide a common registration/probe API so the same clkref model
>>> can be reused regardless of where clkref_en registers are placed, e.g.
>>> TCSR on glymur and TLMM on SM8750.
>>>

[...]

>>> +
>>> +static int qcom_clk_ref_is_enabled(struct clk_hw *hw)
>>> +{
>>> +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
>>> +	u32 val;
>>> +	int ret;
>>> +
>>> +	ret = regmap_read(rclk->regmap, rclk->desc.offset, &val);
>>> +	if (ret)
>>> +		return ret;
>>
>> regmap_read returns a negative error code on failure, but the
>> clk_ops.is_enabled() treats the non-zero value as enabled.
>>
> 
> A regmap_read failure doesn't mean the clock is disabled.

Do we have special reason to treat the error number as "true"? Its 
worthy to add a comment to explain why.

Thanks,
Jie

> 
> - Qiang Yu
>> Thanks,
>> Jie
>>
>>> +
>>> +	return !!(val & QCOM_CLK_REF_EN_MASK);
>>> +}
>>> +
>>> +static const struct clk_ops qcom_clk_ref_ops = {
>>> +	.prepare = qcom_clk_ref_prepare,
>>> +	.unprepare = qcom_clk_ref_unprepare,
>>> +	.enable = qcom_clk_ref_enable,
>>> +	.disable = qcom_clk_ref_disable,
>>> +	.is_enabled = qcom_clk_ref_is_enabled,
>>> +};
>>> +
>>> +static int qcom_clk_ref_register(struct device *dev, struct regmap *regmap,
>>> +				 struct qcom_clk_ref *clk_refs,
>>> +				 const struct qcom_clk_ref_desc *descs,
>>> +				 size_t num_clk_refs)
>>> +{
>>> +	const struct qcom_clk_ref_desc *desc;
>>> +	struct qcom_clk_ref *clk_ref;
>>> +	size_t clk_idx;
>>> +	unsigned int i;
>>> +	int ret;
>>> +
>>> +	for (clk_idx = 0; clk_idx < num_clk_refs; clk_idx++) {
>>> +		clk_ref = &clk_refs[clk_idx];
>>> +		desc = &descs[clk_idx];
>>> +
>>> +		if (!desc->name)
>>> +			continue;
>>> +
>>> +		clk_ref->regmap = regmap;
>>> +		clk_ref->desc = *desc;
>>> +
>>> +		if (clk_ref->desc.num_regulators) {
>>> +			clk_ref->regulators = devm_kcalloc(dev, clk_ref->desc.num_regulators,
>>> +							   sizeof(*clk_ref->regulators),
>>> +							   GFP_KERNEL);
>>> +			if (!clk_ref->regulators)
>>> +				return -ENOMEM;
>>> +
>>> +			for (i = 0; i < clk_ref->desc.num_regulators; i++)
>>> +				clk_ref->regulators[i].supply =
>>> +					clk_ref->desc.regulator_names[i];
>>> +
>>> +			ret = devm_regulator_bulk_get(dev, clk_ref->desc.num_regulators,
>>> +						      clk_ref->regulators);
>>> +			if (ret)
>>> +				return dev_err_probe(dev, ret,
>>> +						     "Failed to get regulators for %s\n",
>>> +						     clk_ref->desc.name);
>>> +		}
>>> +
>>> +		clk_ref->init_data.name = clk_ref->desc.name;
>>> +		clk_ref->init_data.parent_data = &qcom_clk_ref_parent_data;
>>> +		clk_ref->init_data.num_parents = 1;
>>> +		clk_ref->init_data.ops = &qcom_clk_ref_ops;
>>> +		clk_ref->hw.init = &clk_ref->init_data;
>>> +
>>> +		ret = devm_clk_hw_register(dev, &clk_ref->hw);
>>> +		if (ret)
>>> +			return ret;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static struct clk_hw *qcom_clk_ref_provider_get(struct of_phandle_args *clkspec, void *data)
>>> +{
>>> +	struct qcom_clk_ref_provider *provider = data;
>>> +	unsigned int idx = clkspec->args[0];
>>> +
>>> +	if (idx >= provider->num_refs)
>>> +		return ERR_PTR(-EINVAL);
>>> +
>>> +	if (!provider->refs[idx].regmap)
>>> +		return ERR_PTR(-ENOENT);
>>> +
>>> +	return &provider->refs[idx].hw;
>>> +}
>>> +
>>> +int qcom_clk_ref_probe(struct platform_device *pdev,
>>> +		       const struct regmap_config *config,
>>> +		       const struct qcom_clk_ref_desc *descs,
>>> +		       size_t num_clk_refs)
>>> +{
>>> +	struct qcom_clk_ref_provider *provider;
>>> +	struct device *dev = &pdev->dev;
>>> +	struct regmap *regmap;
>>> +	void __iomem *base;
>>> +	int ret;
>>> +
>>> +	base = devm_platform_ioremap_resource(pdev, 0);
>>> +	if (IS_ERR(base))
>>> +		return PTR_ERR(base);
>>> +
>>> +	regmap = devm_regmap_init_mmio(dev, base, config);
>>> +	if (IS_ERR(regmap))
>>> +		return PTR_ERR(regmap);
>>> +
>>> +	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
>>> +	if (!provider)
>>> +		return -ENOMEM;
>>> +
>>> +	provider->refs = devm_kcalloc(dev, num_clk_refs, sizeof(*provider->refs),
>>> +				      GFP_KERNEL);
>>> +	if (!provider->refs)
>>> +		return -ENOMEM;
>>> +
>>> +	provider->num_refs = num_clk_refs;
>>> +
>>> +	ret = qcom_clk_ref_register(dev, regmap, provider->refs, descs,
>>> +				    provider->num_refs);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	return devm_of_clk_add_hw_provider(dev, qcom_clk_ref_provider_get, provider);
>>> +}
>>> +EXPORT_SYMBOL_GPL(qcom_clk_ref_probe);
>>> diff --git a/include/linux/clk/qcom.h b/include/linux/clk/qcom.h
>>> new file mode 100644
>>> index 000000000000..09e2e3178cfb
>>> --- /dev/null
>>> +++ b/include/linux/clk/qcom.h
>>> @@ -0,0 +1,69 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * Copyright (c) 2026, Qualcomm Technologies, Inc. and/or its subsidiaries.
>>> + */
>>> +
>>> +#ifndef __LINUX_CLK_QCOM_H
>>> +#define __LINUX_CLK_QCOM_H
>>> +
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/errno.h>
>>> +#include <linux/kconfig.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/types.h>
>>> +
>>> +struct device;
>>> +struct platform_device;
>>> +struct regulator_bulk_data;
>>> +
>>> +/**
>>> + * struct qcom_clk_ref_desc - descriptor for a clkref_en gate clock
>>> + * @name: clock name exposed to the common clock framework
>>> + * @offset: clkref_en register offset from the block base
>>> + * @regulator_names: optional supply names enabled while preparing the clock
>>> + * @num_regulators: number of entries in @regulator_names
>>> + */
>>> +struct qcom_clk_ref_desc {
>>> +	const char *name;
>>> +	u32 offset;
>>> +	const char * const *regulator_names;
>>> +	unsigned int num_regulators;
>>> +};
>>> +
>>> +/**
>>> + * struct qcom_clk_ref - per-clock data for a clkref_en gate clock
>>> + * @hw: common clock framework hardware clock handle
>>> + * @init_data: common clock framework registration data
>>> + * @regmap: register map backing the clkref_en register
>>> + * @desc: clock descriptor copied at registration time
>>> + * @regulators: optional bulk regulator handles for @desc.regulator_names
>>> + */
>>> +struct qcom_clk_ref {
>>> +	struct clk_hw hw;
>>> +	struct clk_init_data init_data;
>>> +	struct regmap *regmap;
>>> +	struct qcom_clk_ref_desc desc;
>>> +	struct regulator_bulk_data *regulators;
>>> +};
>>> +
>>> +#if IS_ENABLED(CONFIG_COMMON_CLK_QCOM)
>>> +
>>> +int qcom_clk_ref_probe(struct platform_device *pdev,
>>> +		       const struct regmap_config *config,
>>> +		       const struct qcom_clk_ref_desc *descs,
>>> +		       size_t num_clk_refs);
>>> +
>>> +#else
>>> +
>>> +static inline int
>>> +qcom_clk_ref_probe(struct platform_device *pdev,
>>> +		   const struct regmap_config *config,
>>> +		   const struct qcom_clk_ref_desc *descs,
>>> +		   size_t num_clk_refs)
>>> +{
>>> +	return -EOPNOTSUPP;
>>> +}
>>> +
>>> +#endif
>>> +
>>> +#endif
>>>
>>

Re: [PATCH v4 2/7] clk: qcom: Add generic clkref_en support

[Dmitry Baryshkov]

On Thu, May 28, 2026 at 09:46:51PM +0800, Jie Gan wrote:
> 
> 
> On 5/28/2026 9:06 PM, Qiang Yu wrote:
> > On Thu, May 28, 2026 at 11:03:45AM +0800, Jie Gan wrote:
> > > 
> > > 
> > > On 5/28/2026 10:29 AM, Qiang Yu wrote:
> > > > Before XO refclk is distributed to PCIe/USB/eDP PHYs, it passes through
> > > > a QREF block. QREF is powered by dedicated LDO rails, and the clkref_en
> > > > register controls whether refclk is gated through to the PHY side.
> > > > 
> > > > These clkref controls are different from typical GCC branch clocks:
> > > > - only a single enable bit is present, without branch-style config bits
> > > > - regulators must be voted before enable and unvoted after disable
> > > > 
> > > > Model this as a dedicated clk_ref clock type with custom clk_ops instead
> > > > of reusing struct clk_branch semantics.
> > > > 
> > > > Also provide a common registration/probe API so the same clkref model
> > > > can be reused regardless of where clkref_en registers are placed, e.g.
> > > > TCSR on glymur and TLMM on SM8750.
> > > > 
> 
> [...]
> 
> > > > +
> > > > +static int qcom_clk_ref_is_enabled(struct clk_hw *hw)
> > > > +{
> > > > +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> > > > +	u32 val;
> > > > +	int ret;
> > > > +
> > > > +	ret = regmap_read(rclk->regmap, rclk->desc.offset, &val);
> > > > +	if (ret)
> > > > +		return ret;
> > > 
> > > regmap_read returns a negative error code on failure, but the
> > > clk_ops.is_enabled() treats the non-zero value as enabled.
> > > 
> > 
> > A regmap_read failure doesn't mean the clock is disabled.
> 
> Do we have special reason to treat the error number as "true"? Its worthy to
> add a comment to explain why.

to be 'false', the error number must be 0.


-- 
With best wishes
Dmitry

Re: [PATCH v4 2/7] clk: qcom: Add generic clkref_en support

[Qiang Yu]

On Thu, May 28, 2026 at 06:01:31PM +0300, Dmitry Baryshkov wrote:
> On Thu, May 28, 2026 at 09:46:51PM +0800, Jie Gan wrote:
> > 
> > 
> > On 5/28/2026 9:06 PM, Qiang Yu wrote:
> > > On Thu, May 28, 2026 at 11:03:45AM +0800, Jie Gan wrote:
> > > > 
> > > > 
> > > > On 5/28/2026 10:29 AM, Qiang Yu wrote:
> > > > > Before XO refclk is distributed to PCIe/USB/eDP PHYs, it passes through
> > > > > a QREF block. QREF is powered by dedicated LDO rails, and the clkref_en
> > > > > register controls whether refclk is gated through to the PHY side.
> > > > > 
> > > > > These clkref controls are different from typical GCC branch clocks:
> > > > > - only a single enable bit is present, without branch-style config bits
> > > > > - regulators must be voted before enable and unvoted after disable
> > > > > 
> > > > > Model this as a dedicated clk_ref clock type with custom clk_ops instead
> > > > > of reusing struct clk_branch semantics.
> > > > > 
> > > > > Also provide a common registration/probe API so the same clkref model
> > > > > can be reused regardless of where clkref_en registers are placed, e.g.
> > > > > TCSR on glymur and TLMM on SM8750.
> > > > > 
> > 
> > [...]
> > 
> > > > > +
> > > > > +static int qcom_clk_ref_is_enabled(struct clk_hw *hw)
> > > > > +{
> > > > > +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> > > > > +	u32 val;
> > > > > +	int ret;
> > > > > +
> > > > > +	ret = regmap_read(rclk->regmap, rclk->desc.offset, &val);
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > 
> > > > regmap_read returns a negative error code on failure, but the
> > > > clk_ops.is_enabled() treats the non-zero value as enabled.
> > > > 
> > > 
> > > A regmap_read failure doesn't mean the clock is disabled.
> > 
> > Do we have special reason to treat the error number as "true"? Its worthy to
> > add a comment to explain why.
> 
> to be 'false', the error number must be 0.
>

Okay, will change it in next version.

- Qiang Yu
> 
> -- 
> With best wishes
> Dmitry

Re: [PATCH v4 2/7] clk: qcom: Add generic clkref_en support

[Qiang Yu]

On Thu, May 28, 2026 at 09:46:51PM +0800, Jie Gan wrote:
> 
> 
> On 5/28/2026 9:06 PM, Qiang Yu wrote:
> > On Thu, May 28, 2026 at 11:03:45AM +0800, Jie Gan wrote:
> > > 
> > > 
> > > On 5/28/2026 10:29 AM, Qiang Yu wrote:
> > > > Before XO refclk is distributed to PCIe/USB/eDP PHYs, it passes through
> > > > a QREF block. QREF is powered by dedicated LDO rails, and the clkref_en
> > > > register controls whether refclk is gated through to the PHY side.
> > > > 
> > > > These clkref controls are different from typical GCC branch clocks:
> > > > - only a single enable bit is present, without branch-style config bits
> > > > - regulators must be voted before enable and unvoted after disable
> > > > 
> > > > Model this as a dedicated clk_ref clock type with custom clk_ops instead
> > > > of reusing struct clk_branch semantics.
> > > > 
> > > > Also provide a common registration/probe API so the same clkref model
> > > > can be reused regardless of where clkref_en registers are placed, e.g.
> > > > TCSR on glymur and TLMM on SM8750.
> > > > 
> 
> [...]
> 
> > > > +
> > > > +static int qcom_clk_ref_is_enabled(struct clk_hw *hw)
> > > > +{
> > > > +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> > > > +	u32 val;
> > > > +	int ret;
> > > > +
> > > > +	ret = regmap_read(rclk->regmap, rclk->desc.offset, &val);
> > > > +	if (ret)
> > > > +		return ret;
> > > 
> > > regmap_read returns a negative error code on failure, but the
> > > clk_ops.is_enabled() treats the non-zero value as enabled.
> > > 
> > 
> > A regmap_read failure doesn't mean the clock is disabled.
> 
> Do we have special reason to treat the error number as "true"? Its worthy to
> add a comment to explain why.
>
I'm not sure. In some clk drivers, they return fail. eg.clk-branch.c

- Qiang Yu
> Thanks,
> Jie
> 
> > 
> > - Qiang Yu
> > > Thanks,
> > > Jie
> > > 
> > > > +
> > > > +	return !!(val & QCOM_CLK_REF_EN_MASK);
> > > > +}
> > > > +
> > > > +static const struct clk_ops qcom_clk_ref_ops = {
> > > > +	.prepare = qcom_clk_ref_prepare,
> > > > +	.unprepare = qcom_clk_ref_unprepare,
> > > > +	.enable = qcom_clk_ref_enable,
> > > > +	.disable = qcom_clk_ref_disable,
> > > > +	.is_enabled = qcom_clk_ref_is_enabled,
> > > > +};
> > > > +
> > > > +static int qcom_clk_ref_register(struct device *dev, struct regmap *regmap,
> > > > +				 struct qcom_clk_ref *clk_refs,
> > > > +				 const struct qcom_clk_ref_desc *descs,
> > > > +				 size_t num_clk_refs)
> > > > +{
> > > > +	const struct qcom_clk_ref_desc *desc;
> > > > +	struct qcom_clk_ref *clk_ref;
> > > > +	size_t clk_idx;
> > > > +	unsigned int i;
> > > > +	int ret;
> > > > +
> > > > +	for (clk_idx = 0; clk_idx < num_clk_refs; clk_idx++) {
> > > > +		clk_ref = &clk_refs[clk_idx];
> > > > +		desc = &descs[clk_idx];
> > > > +
> > > > +		if (!desc->name)
> > > > +			continue;
> > > > +
> > > > +		clk_ref->regmap = regmap;
> > > > +		clk_ref->desc = *desc;
> > > > +
> > > > +		if (clk_ref->desc.num_regulators) {
> > > > +			clk_ref->regulators = devm_kcalloc(dev, clk_ref->desc.num_regulators,
> > > > +							   sizeof(*clk_ref->regulators),
> > > > +							   GFP_KERNEL);
> > > > +			if (!clk_ref->regulators)
> > > > +				return -ENOMEM;
> > > > +
> > > > +			for (i = 0; i < clk_ref->desc.num_regulators; i++)
> > > > +				clk_ref->regulators[i].supply =
> > > > +					clk_ref->desc.regulator_names[i];
> > > > +
> > > > +			ret = devm_regulator_bulk_get(dev, clk_ref->desc.num_regulators,
> > > > +						      clk_ref->regulators);
> > > > +			if (ret)
> > > > +				return dev_err_probe(dev, ret,
> > > > +						     "Failed to get regulators for %s\n",
> > > > +						     clk_ref->desc.name);
> > > > +		}
> > > > +
> > > > +		clk_ref->init_data.name = clk_ref->desc.name;
> > > > +		clk_ref->init_data.parent_data = &qcom_clk_ref_parent_data;
> > > > +		clk_ref->init_data.num_parents = 1;
> > > > +		clk_ref->init_data.ops = &qcom_clk_ref_ops;
> > > > +		clk_ref->hw.init = &clk_ref->init_data;
> > > > +
> > > > +		ret = devm_clk_hw_register(dev, &clk_ref->hw);
> > > > +		if (ret)
> > > > +			return ret;
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static struct clk_hw *qcom_clk_ref_provider_get(struct of_phandle_args *clkspec, void *data)
> > > > +{
> > > > +	struct qcom_clk_ref_provider *provider = data;
> > > > +	unsigned int idx = clkspec->args[0];
> > > > +
> > > > +	if (idx >= provider->num_refs)
> > > > +		return ERR_PTR(-EINVAL);
> > > > +
> > > > +	if (!provider->refs[idx].regmap)
> > > > +		return ERR_PTR(-ENOENT);
> > > > +
> > > > +	return &provider->refs[idx].hw;
> > > > +}
> > > > +
> > > > +int qcom_clk_ref_probe(struct platform_device *pdev,
> > > > +		       const struct regmap_config *config,
> > > > +		       const struct qcom_clk_ref_desc *descs,
> > > > +		       size_t num_clk_refs)
> > > > +{
> > > > +	struct qcom_clk_ref_provider *provider;
> > > > +	struct device *dev = &pdev->dev;
> > > > +	struct regmap *regmap;
> > > > +	void __iomem *base;
> > > > +	int ret;
> > > > +
> > > > +	base = devm_platform_ioremap_resource(pdev, 0);
> > > > +	if (IS_ERR(base))
> > > > +		return PTR_ERR(base);
> > > > +
> > > > +	regmap = devm_regmap_init_mmio(dev, base, config);
> > > > +	if (IS_ERR(regmap))
> > > > +		return PTR_ERR(regmap);
> > > > +
> > > > +	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
> > > > +	if (!provider)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	provider->refs = devm_kcalloc(dev, num_clk_refs, sizeof(*provider->refs),
> > > > +				      GFP_KERNEL);
> > > > +	if (!provider->refs)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	provider->num_refs = num_clk_refs;
> > > > +
> > > > +	ret = qcom_clk_ref_register(dev, regmap, provider->refs, descs,
> > > > +				    provider->num_refs);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	return devm_of_clk_add_hw_provider(dev, qcom_clk_ref_provider_get, provider);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(qcom_clk_ref_probe);
> > > > diff --git a/include/linux/clk/qcom.h b/include/linux/clk/qcom.h
> > > > new file mode 100644
> > > > index 000000000000..09e2e3178cfb
> > > > --- /dev/null
> > > > +++ b/include/linux/clk/qcom.h
> > > > @@ -0,0 +1,69 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +/*
> > > > + * Copyright (c) 2026, Qualcomm Technologies, Inc. and/or its subsidiaries.
> > > > + */
> > > > +
> > > > +#ifndef __LINUX_CLK_QCOM_H
> > > > +#define __LINUX_CLK_QCOM_H
> > > > +
> > > > +#include <linux/clk-provider.h>
> > > > +#include <linux/errno.h>
> > > > +#include <linux/kconfig.h>
> > > > +#include <linux/regmap.h>
> > > > +#include <linux/types.h>
> > > > +
> > > > +struct device;
> > > > +struct platform_device;
> > > > +struct regulator_bulk_data;
> > > > +
> > > > +/**
> > > > + * struct qcom_clk_ref_desc - descriptor for a clkref_en gate clock
> > > > + * @name: clock name exposed to the common clock framework
> > > > + * @offset: clkref_en register offset from the block base
> > > > + * @regulator_names: optional supply names enabled while preparing the clock
> > > > + * @num_regulators: number of entries in @regulator_names
> > > > + */
> > > > +struct qcom_clk_ref_desc {
> > > > +	const char *name;
> > > > +	u32 offset;
> > > > +	const char * const *regulator_names;
> > > > +	unsigned int num_regulators;
> > > > +};
> > > > +
> > > > +/**
> > > > + * struct qcom_clk_ref - per-clock data for a clkref_en gate clock
> > > > + * @hw: common clock framework hardware clock handle
> > > > + * @init_data: common clock framework registration data
> > > > + * @regmap: register map backing the clkref_en register
> > > > + * @desc: clock descriptor copied at registration time
> > > > + * @regulators: optional bulk regulator handles for @desc.regulator_names
> > > > + */
> > > > +struct qcom_clk_ref {
> > > > +	struct clk_hw hw;
> > > > +	struct clk_init_data init_data;
> > > > +	struct regmap *regmap;
> > > > +	struct qcom_clk_ref_desc desc;
> > > > +	struct regulator_bulk_data *regulators;
> > > > +};
> > > > +
> > > > +#if IS_ENABLED(CONFIG_COMMON_CLK_QCOM)
> > > > +
> > > > +int qcom_clk_ref_probe(struct platform_device *pdev,
> > > > +		       const struct regmap_config *config,
> > > > +		       const struct qcom_clk_ref_desc *descs,
> > > > +		       size_t num_clk_refs);
> > > > +
> > > > +#else
> > > > +
> > > > +static inline int
> > > > +qcom_clk_ref_probe(struct platform_device *pdev,
> > > > +		   const struct regmap_config *config,
> > > > +		   const struct qcom_clk_ref_desc *descs,
> > > > +		   size_t num_clk_refs)
> > > > +{
> > > > +	return -EOPNOTSUPP;
> > > > +}
> > > > +
> > > > +#endif
> > > > +
> > > > +#endif
> > > > 
> > > 
> 

Re: [PATCH v4 2/7] clk: qcom: Add generic clkref_en support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Unbalanced regulator disable during late init for clocks left enabled by the bootloader.
- [Medium] Hardcoded resource mapping and OF provider registration prevent API reuse in multi-functional drivers.
- [Medium] `qcom_clk_ref_is_enabled` returns a negative error code on failure, which CCF interprets as true.
--

commit 958034c0adafc3f72b4d7e46df81935d74404f7a
Author: Qiang Yu <qiang.yu@oss.qualcomm.com>

clk: qcom: Add generic clkref_en support

This commit introduces a new clk_ref clock type with custom clk_ops for
QREF blocks. It provides a common registration and probe API intended to
be reused across different placements like TCSR and TLMM.

> diff --git a/drivers/clk/qcom/clk-ref.c b/drivers/clk/qcom/clk-ref.c
> new file mode 100644
> index 0000000000000..213c0f58bb36e
> --- /dev/null
> +++ b/drivers/clk/qcom/clk-ref.c

[ ... ]

> +static void qcom_clk_ref_unprepare(struct clk_hw *hw)
> +{
> +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> +
> +	if (rclk->desc.num_regulators)
> +		regulator_bulk_disable(rclk->desc.num_regulators, rclk->regulators);
> +}

[Severity: High]
If the bootloader leaves this clock enabled, the common clock framework
will call unprepare() during late init to turn off unused clocks. Since
prepare() was never called by the kernel, won't this
regulator_bulk_disable() call trigger an unbalanced disable warning and
permanently leak power?

Does the driver need to sync the hardware state during probe by enabling
the regulators if the clock is already on?

[ ... ]

> +static int qcom_clk_ref_is_enabled(struct clk_hw *hw)
> +{
> +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read(rclk->regmap, rclk->desc.offset, &val);
> +	if (ret)
> +		return ret;

[Severity: Medium]
If regmap_read() fails, this returns a negative error code (e.g., -EINVAL).
Since the common clock framework evaluates the is_enabled callback as a
boolean, won't it treat this negative error code as true and incorrectly
assume the clock is enabled?

> +
> +	return !!(val & QCOM_CLK_REF_EN_MASK);
> +}

[ ... ]

> +int qcom_clk_ref_probe(struct platform_device *pdev,
> +		       const struct regmap_config *config,
> +		       const struct qcom_clk_ref_desc *descs,
> +		       size_t num_clk_refs)
> +{
> +	struct qcom_clk_ref_provider *provider;
> +	struct device *dev = &pdev->dev;
> +	struct regmap *regmap;
> +	void __iomem *base;
> +	int ret;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);

[Severity: Medium]
The commit message mentions reusing this model for blocks like TLMM on SM8750.
If a multi-functional driver like TLMM tries to use this helper, won't
devm_platform_ioremap_resource() fail with -EBUSY because the parent driver
already maps the register region?

Since qcom_clk_ref_register() is static and callers are forced to use
qcom_clk_ref_probe(), does this hardcoded resource mapping prevent the
intended integration with multi-functional drivers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527-tcsr_qref_0527-v4-0-ded83866c9d9@oss.qualcomm.com?part=2

Re: [PATCH v4 2/7] clk: qcom: Add generic clkref_en support

[Qiang Yu]

On 5/28/2026 11:05 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Unbalanced regulator disable during late init for clocks left enabled by the bootloader.
> - [Medium] Hardcoded resource mapping and OF provider registration prevent API reuse in multi-functional drivers.
> - [Medium] `qcom_clk_ref_is_enabled` returns a negative error code on failure, which CCF interprets as true.
> --
>
> commit 958034c0adafc3f72b4d7e46df81935d74404f7a
> Author: Qiang Yu <qiang.yu@oss.qualcomm.com>
>
> clk: qcom: Add generic clkref_en support
>
> This commit introduces a new clk_ref clock type with custom clk_ops for
> QREF blocks. It provides a common registration and probe API intended to
> be reused across different placements like TCSR and TLMM.
>
>> diff --git a/drivers/clk/qcom/clk-ref.c b/drivers/clk/qcom/clk-ref.c
>> new file mode 100644
>> index 0000000000000..213c0f58bb36e
>> --- /dev/null
>> +++ b/drivers/clk/qcom/clk-ref.c
> [ ... ]
>
>> +static void qcom_clk_ref_unprepare(struct clk_hw *hw)
>> +{
>> +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
>> +
>> +	if (rclk->desc.num_regulators)
>> +		regulator_bulk_disable(rclk->desc.num_regulators, rclk->regulators);
>> +}
> [Severity: High]
> If the bootloader leaves this clock enabled, the common clock framework
> will call unprepare() during late init to turn off unused clocks. Since
> prepare() was never called by the kernel, won't this
> regulator_bulk_disable() call trigger an unbalanced disable warning and
> permanently leak power?
>
> Does the driver need to sync the hardware state during probe by enabling
> the regulators if the clock is already on?
clk_unprepare_unused_subtree() calls unprepare() only when
clk_core_is_prepared() returns true. Since our driver has no .is_prepared
op, the framework falls back to checking core->prepare_count, which is 0
if the kernel never called prepare(). So unprepare() will not be called in
that path, and the regulator imbalance you describe cannot happen via the
unused clock cleanup.

- Qiang Yu
>
> [ ... ]
>
>> +static int qcom_clk_ref_is_enabled(struct clk_hw *hw)
>> +{
>> +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
>> +	u32 val;
>> +	int ret;
>> +
>> +	ret = regmap_read(rclk->regmap, rclk->desc.offset, &val);
>> +	if (ret)
>> +		return ret;
> [Severity: Medium]
> If regmap_read() fails, this returns a negative error code (e.g., -EINVAL).
> Since the common clock framework evaluates the is_enabled callback as a
> boolean, won't it treat this negative error code as true and incorrectly
> assume the clock is enabled?
>
>> +
>> +	return !!(val & QCOM_CLK_REF_EN_MASK);
>> +}
> [ ... ]
>
>> +int qcom_clk_ref_probe(struct platform_device *pdev,
>> +		       const struct regmap_config *config,
>> +		       const struct qcom_clk_ref_desc *descs,
>> +		       size_t num_clk_refs)
>> +{
>> +	struct qcom_clk_ref_provider *provider;
>> +	struct device *dev = &pdev->dev;
>> +	struct regmap *regmap;
>> +	void __iomem *base;
>> +	int ret;
>> +
>> +	base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(base))
>> +		return PTR_ERR(base);
> [Severity: Medium]
> The commit message mentions reusing this model for blocks like TLMM on SM8750.
> If a multi-functional driver like TLMM tries to use this helper, won't
> devm_platform_ioremap_resource() fail with -EBUSY because the parent driver
> already maps the register region?
>
> Since qcom_clk_ref_register() is static and callers are forced to use
> qcom_clk_ref_probe(), does this hardcoded resource mapping prevent the
> intended integration with multi-functional drivers?
>

Re: [PATCH v4 2/7] clk: qcom: Add generic clkref_en support

[Konrad Dybcio]

On 5/28/26 4:29 AM, Qiang Yu wrote:
> Before XO refclk is distributed to PCIe/USB/eDP PHYs, it passes through
> a QREF block. QREF is powered by dedicated LDO rails, and the clkref_en
> register controls whether refclk is gated through to the PHY side.
> 
> These clkref controls are different from typical GCC branch clocks:
> - only a single enable bit is present, without branch-style config bits
> - regulators must be voted before enable and unvoted after disable
> 
> Model this as a dedicated clk_ref clock type with custom clk_ops instead
> of reusing struct clk_branch semantics.
> 
> Also provide a common registration/probe API so the same clkref model
> can be reused regardless of where clkref_en registers are placed, e.g.
> TCSR on glymur and TLMM on SM8750.
> 
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> ---

[...]

> +struct qcom_clk_ref {
> +	struct clk_hw hw;
> +	struct clk_init_data init_data;

We don't need init_data for each one of these, we can construct it in
probe scope:

struct clk_init_data init_data = { };

init_data.name = clk_ref->desc.name;
init_data.parent_data = &qcom_clk_ref_parent_data;
init_data.num_parents = 1;
init_data.ops = &qcom_clk_ref_ops;
clk_ref->hw.init = &init_data;

ret = devm_clk_hw_register(dev, hw);
// not needed past that point, __clk_register zeroes hw->init internally

Konrad

Re: [PATCH v4 2/7] clk: qcom: Add generic clkref_en support

[Qiang Yu]

On Tue, Jun 09, 2026 at 02:38:08PM +0200, Konrad Dybcio wrote:
> On 5/28/26 4:29 AM, Qiang Yu wrote:
> > Before XO refclk is distributed to PCIe/USB/eDP PHYs, it passes through
> > a QREF block. QREF is powered by dedicated LDO rails, and the clkref_en
> > register controls whether refclk is gated through to the PHY side.
> > 
> > These clkref controls are different from typical GCC branch clocks:
> > - only a single enable bit is present, without branch-style config bits
> > - regulators must be voted before enable and unvoted after disable
> > 
> > Model this as a dedicated clk_ref clock type with custom clk_ops instead
> > of reusing struct clk_branch semantics.
> > 
> > Also provide a common registration/probe API so the same clkref model
> > can be reused regardless of where clkref_en registers are placed, e.g.
> > TCSR on glymur and TLMM on SM8750.
> > 
> > Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> > ---
> 
> [...]
> 
> > +struct qcom_clk_ref {
> > +	struct clk_hw hw;
> > +	struct clk_init_data init_data;
> 
> We don't need init_data for each one of these, we can construct it in
> probe scope:
> 
> struct clk_init_data init_data = { };
> 
> init_data.name = clk_ref->desc.name;
> init_data.parent_data = &qcom_clk_ref_parent_data;
> init_data.num_parents = 1;
> init_data.ops = &qcom_clk_ref_ops;
> clk_ref->hw.init = &init_data;
> 
> ret = devm_clk_hw_register(dev, hw);
> // not needed past that point, __clk_register zeroes hw->init internally
>

Thanks, will fix in next version.

- Qiang Yu
> Konrad

Re: [PATCH v4 2/7] clk: qcom: Add generic clkref_en support

[Konrad Dybcio]

On 5/28/26 4:29 AM, Qiang Yu wrote:
> Before XO refclk is distributed to PCIe/USB/eDP PHYs, it passes through
> a QREF block. QREF is powered by dedicated LDO rails, and the clkref_en
> register controls whether refclk is gated through to the PHY side.
> 
> These clkref controls are different from typical GCC branch clocks:
> - only a single enable bit is present, without branch-style config bits
> - regulators must be voted before enable and unvoted after disable
> 
> Model this as a dedicated clk_ref clock type with custom clk_ops instead
> of reusing struct clk_branch semantics.
> 
> Also provide a common registration/probe API so the same clkref model
> can be reused regardless of where clkref_en registers are placed, e.g.
> TCSR on glymur and TLMM on SM8750.
> 
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> ---

[...]

> +static int qcom_clk_ref_enable(struct clk_hw *hw)
> +{
> +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> +	int ret;
> +
> +	ret = regmap_update_bits(rclk->regmap, rclk->desc.offset, QCOM_CLK_REF_EN_MASK,
> +				 QCOM_CLK_REF_EN_MASK);

regmap_set_bits()

> +	if (ret)
> +		return ret;
> +
> +	udelay(10);
> +
> +	return 0;
> +}
> +
> +static void qcom_clk_ref_disable(struct clk_hw *hw)
> +{
> +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> +
> +	regmap_update_bits(rclk->regmap, rclk->desc.offset, QCOM_CLK_REF_EN_MASK, 0);

regmap_clear_bits()

[...]

> +static int qcom_clk_ref_register(struct device *dev, struct regmap *regmap,
> +				 struct qcom_clk_ref *clk_refs,
> +				 const struct qcom_clk_ref_desc *descs,
> +				 size_t num_clk_refs)
> +{
> +	const struct qcom_clk_ref_desc *desc;
> +	struct qcom_clk_ref *clk_ref;
> +	size_t clk_idx;
> +	unsigned int i;
> +	int ret;
> +
> +	for (clk_idx = 0; clk_idx < num_clk_refs; clk_idx++) {
> +		clk_ref = &clk_refs[clk_idx];
> +		desc = &descs[clk_idx];
> +
> +		if (!desc->name)
> +			continue;

// this allows "holes" in dt-bindings for $reasons
if (!desc)
	continue;

// this makes sure the programmer did not omit something important
// while not taking the entire system down
if (WARN_ON(!desc->name)
	continue;


Konrad

Re: [PATCH v4 2/7] clk: qcom: Add generic clkref_en support

[Qiang Yu]

On Tue, Jun 09, 2026 at 02:57:52PM +0200, Konrad Dybcio wrote:
> On 5/28/26 4:29 AM, Qiang Yu wrote:
> > Before XO refclk is distributed to PCIe/USB/eDP PHYs, it passes through
> > a QREF block. QREF is powered by dedicated LDO rails, and the clkref_en
> > register controls whether refclk is gated through to the PHY side.
> > 
> > These clkref controls are different from typical GCC branch clocks:
> > - only a single enable bit is present, without branch-style config bits
> > - regulators must be voted before enable and unvoted after disable
> > 
> > Model this as a dedicated clk_ref clock type with custom clk_ops instead
> > of reusing struct clk_branch semantics.
> > 
> > Also provide a common registration/probe API so the same clkref model
> > can be reused regardless of where clkref_en registers are placed, e.g.
> > TCSR on glymur and TLMM on SM8750.
> > 
> > Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> > ---
> 
> [...]
> 
> > +static int qcom_clk_ref_enable(struct clk_hw *hw)
> > +{
> > +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> > +	int ret;
> > +
> > +	ret = regmap_update_bits(rclk->regmap, rclk->desc.offset, QCOM_CLK_REF_EN_MASK,
> > +				 QCOM_CLK_REF_EN_MASK);
> 
> regmap_set_bits()
> 
> > +	if (ret)
> > +		return ret;
> > +
> > +	udelay(10);
> > +
> > +	return 0;
> > +}
> > +
> > +static void qcom_clk_ref_disable(struct clk_hw *hw)
> > +{
> > +	struct qcom_clk_ref *rclk = to_qcom_clk_ref(hw);
> > +
> > +	regmap_update_bits(rclk->regmap, rclk->desc.offset, QCOM_CLK_REF_EN_MASK, 0);
> 
> regmap_clear_bits()
> 
> [...]
>

Will use regmap_clear_bits in next version.

> > +static int qcom_clk_ref_register(struct device *dev, struct regmap *regmap,
> > +				 struct qcom_clk_ref *clk_refs,
> > +				 const struct qcom_clk_ref_desc *descs,
> > +				 size_t num_clk_refs)
> > +{
> > +	const struct qcom_clk_ref_desc *desc;
> > +	struct qcom_clk_ref *clk_ref;
> > +	size_t clk_idx;
> > +	unsigned int i;
> > +	int ret;
> > +
> > +	for (clk_idx = 0; clk_idx < num_clk_refs; clk_idx++) {
> > +		clk_ref = &clk_refs[clk_idx];
> > +		desc = &descs[clk_idx];
> > +
> > +		if (!desc->name)
> > +			continue;
> 
> // this allows "holes" in dt-bindings for $reasons
> if (!desc)
> 	continue;
> 
> // this makes sure the programmer did not omit something important
> // while not taking the entire system down
> if (WARN_ON(!desc->name)
> 	continue;
>
The NULL name check is intentional - the descriptor array is indexed by
clock ID, and mahua has fewer clocks than glymur, leaving holes at
certain indices. So this is expected at runtime. WARN_ON would be noise
log here.

- Qiang Yu
> 
> Konrad

[PATCH v4 3/7] clk: qcom: tcsrcc-glymur: Migrate tcsr_pcie_N_clkref_en to clk_ref common helper

[Qiang Yu]

Replace local clk_branch-based clkref definitions with descriptor-based
registration via qcom_clk_ref_probe().

This keeps the glymur driver focused on clock metadata and reuses common
runtime logic for regulator handling, enable/disable sequencing, and OF
provider wiring.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 drivers/clk/qcom/tcsrcc-glymur.c | 330 ++++++++++-----------------------------
 1 file changed, 83 insertions(+), 247 deletions(-)

diff --git a/drivers/clk/qcom/tcsrcc-glymur.c b/drivers/clk/qcom/tcsrcc-glymur.c
index 9c0edebcdbb1..e317003398d1 100644
--- a/drivers/clk/qcom/tcsrcc-glymur.c
+++ b/drivers/clk/qcom/tcsrcc-glymur.c
@@ -4,277 +4,111 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/clk/qcom.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/qcom,glymur-tcsr.h>
 
-#include "clk-alpha-pll.h"
-#include "clk-branch.h"
-#include "clk-pll.h"
-#include "clk-rcg.h"
-#include "clk-regmap.h"
-#include "clk-regmap-divider.h"
-#include "clk-regmap-mux.h"
-#include "common.h"
-#include "gdsc.h"
-#include "reset.h"
-
-enum {
-	DT_BI_TCXO_PAD,
+static const char * const glymur_tcsr_tx0_rx5_regulators[] = {
+	"vdda-refgen3-0p9",
+	"vdda-refgen3-1p2",
+	"vdda-qrefrx5-0p9",
+	"vdda-qreftx0-0p9",
+	"vdda-qreftx0-1p2",
 };
 
-static struct clk_branch tcsr_edp_clkref_en = {
-	.halt_reg = 0x60,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x60,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "tcsr_edp_clkref_en",
-			.parent_data = &(const struct clk_parent_data){
-				.index = DT_BI_TCXO_PAD,
-			},
-			.num_parents = 1,
-			.ops = &clk_branch2_ops,
-		},
-	},
+static const char * const glymur_tcsr_tx1_rpt01_rx1_regulators[] = {
+	"vdda-refgen4-0p9",
+	"vdda-refgen4-1p2",
+	"vdda-qreftx1-0p9",
+	"vdda-qrefrpt0-0p9",
+	"vdda-qrefrpt1-0p9",
+	"vdda-qrefrx1-0p9",
 };
 
-static struct clk_branch tcsr_pcie_1_clkref_en = {
-	.halt_reg = 0x48,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x48,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "tcsr_pcie_1_clkref_en",
-			.parent_data = &(const struct clk_parent_data){
-				.index = DT_BI_TCXO_PAD,
-			},
-			.num_parents = 1,
-			.ops = &clk_branch2_ops,
-		},
-	},
+static const char * const glymur_tcsr_tx1_rpt012_rx2_regulators[] = {
+	"vdda-refgen4-0p9",
+	"vdda-refgen4-1p2",
+	"vdda-qreftx1-0p9",
+	"vdda-qrefrpt0-0p9",
+	"vdda-qrefrpt1-0p9",
+	"vdda-qrefrpt2-0p9",
+	"vdda-qrefrx2-0p9",
 };
 
-static struct clk_branch tcsr_pcie_2_clkref_en = {
-	.halt_reg = 0x4c,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x4c,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "tcsr_pcie_2_clkref_en",
-			.parent_data = &(const struct clk_parent_data){
-				.index = DT_BI_TCXO_PAD,
-			},
-			.num_parents = 1,
-			.ops = &clk_branch2_ops,
-		},
-	},
+static const struct regmap_config tcsr_cc_glymur_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0x94,
+	.fast_io = true,
 };
 
-static struct clk_branch tcsr_pcie_3_clkref_en = {
-	.halt_reg = 0x54,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x54,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "tcsr_pcie_3_clkref_en",
-			.parent_data = &(const struct clk_parent_data){
-				.index = DT_BI_TCXO_PAD,
-			},
-			.num_parents = 1,
-			.ops = &clk_branch2_ops,
-		},
+static const struct qcom_clk_ref_desc tcsr_cc_glymur_clk_descs[] = {
+	[TCSR_EDP_CLKREF_EN] = {
+		.name = "tcsr_edp_clkref_en",
+		.offset = 0x60,
 	},
-};
-
-static struct clk_branch tcsr_pcie_4_clkref_en = {
-	.halt_reg = 0x58,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x58,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "tcsr_pcie_4_clkref_en",
-			.parent_data = &(const struct clk_parent_data){
-				.index = DT_BI_TCXO_PAD,
-			},
-			.num_parents = 1,
-			.ops = &clk_branch2_ops,
-		},
+	[TCSR_PCIE_1_CLKREF_EN] = {
+		.name = "tcsr_pcie_1_clkref_en",
+		.offset = 0x48,
+		.regulator_names = glymur_tcsr_tx0_rx5_regulators,
+		.num_regulators = ARRAY_SIZE(glymur_tcsr_tx0_rx5_regulators),
 	},
-};
-
-static struct clk_branch tcsr_usb2_1_clkref_en = {
-	.halt_reg = 0x6c,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x6c,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "tcsr_usb2_1_clkref_en",
-			.parent_data = &(const struct clk_parent_data){
-				.index = DT_BI_TCXO_PAD,
-			},
-			.num_parents = 1,
-			.ops = &clk_branch2_ops,
-		},
+	[TCSR_PCIE_2_CLKREF_EN] = {
+		.name = "tcsr_pcie_2_clkref_en",
+		.offset = 0x4c,
+		.regulator_names = glymur_tcsr_tx1_rpt012_rx2_regulators,
+		.num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt012_rx2_regulators),
 	},
-};
-
-static struct clk_branch tcsr_usb2_2_clkref_en = {
-	.halt_reg = 0x70,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x70,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "tcsr_usb2_2_clkref_en",
-			.parent_data = &(const struct clk_parent_data){
-				.index = DT_BI_TCXO_PAD,
-			},
-			.num_parents = 1,
-			.ops = &clk_branch2_ops,
-		},
+	[TCSR_PCIE_3_CLKREF_EN] = {
+		.name = "tcsr_pcie_3_clkref_en",
+		.offset = 0x54,
+		.regulator_names = glymur_tcsr_tx1_rpt01_rx1_regulators,
+		.num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt01_rx1_regulators),
 	},
-};
-
-static struct clk_branch tcsr_usb2_3_clkref_en = {
-	.halt_reg = 0x74,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x74,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "tcsr_usb2_3_clkref_en",
-			.parent_data = &(const struct clk_parent_data){
-				.index = DT_BI_TCXO_PAD,
-			},
-			.num_parents = 1,
-			.ops = &clk_branch2_ops,
-		},
+	[TCSR_PCIE_4_CLKREF_EN] = {
+		.name = "tcsr_pcie_4_clkref_en",
+		.offset = 0x58,
+		.regulator_names = glymur_tcsr_tx1_rpt012_rx2_regulators,
+		.num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt012_rx2_regulators),
 	},
-};
-
-static struct clk_branch tcsr_usb2_4_clkref_en = {
-	.halt_reg = 0x88,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x88,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "tcsr_usb2_4_clkref_en",
-			.parent_data = &(const struct clk_parent_data){
-				.index = DT_BI_TCXO_PAD,
-			},
-			.num_parents = 1,
-			.ops = &clk_branch2_ops,
-		},
+	[TCSR_USB2_1_CLKREF_EN] = {
+		.name = "tcsr_usb2_1_clkref_en",
+		.offset = 0x6c,
 	},
-};
-
-static struct clk_branch tcsr_usb3_0_clkref_en = {
-	.halt_reg = 0x64,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x64,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "tcsr_usb3_0_clkref_en",
-			.parent_data = &(const struct clk_parent_data){
-				.index = DT_BI_TCXO_PAD,
-			},
-			.num_parents = 1,
-			.ops = &clk_branch2_ops,
-		},
+	[TCSR_USB2_2_CLKREF_EN] = {
+		.name = "tcsr_usb2_2_clkref_en",
+		.offset = 0x70,
 	},
-};
-
-static struct clk_branch tcsr_usb3_1_clkref_en = {
-	.halt_reg = 0x68,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x68,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "tcsr_usb3_1_clkref_en",
-			.parent_data = &(const struct clk_parent_data){
-				.index = DT_BI_TCXO_PAD,
-			},
-			.num_parents = 1,
-			.ops = &clk_branch2_ops,
-		},
+	[TCSR_USB2_3_CLKREF_EN] = {
+		.name = "tcsr_usb2_3_clkref_en",
+		.offset = 0x74,
 	},
-};
-
-static struct clk_branch tcsr_usb4_1_clkref_en = {
-	.halt_reg = 0x44,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x44,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "tcsr_usb4_1_clkref_en",
-			.parent_data = &(const struct clk_parent_data){
-				.index = DT_BI_TCXO_PAD,
-			},
-			.num_parents = 1,
-			.ops = &clk_branch2_ops,
-		},
+	[TCSR_USB2_4_CLKREF_EN] = {
+		.name = "tcsr_usb2_4_clkref_en",
+		.offset = 0x88,
 	},
-};
-
-static struct clk_branch tcsr_usb4_2_clkref_en = {
-	.halt_reg = 0x5c,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x5c,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "tcsr_usb4_2_clkref_en",
-			.parent_data = &(const struct clk_parent_data){
-				.index = DT_BI_TCXO_PAD,
-			},
-			.num_parents = 1,
-			.ops = &clk_branch2_ops,
-		},
+	[TCSR_USB3_0_CLKREF_EN] = {
+		.name = "tcsr_usb3_0_clkref_en",
+		.offset = 0x64,
+	},
+	[TCSR_USB3_1_CLKREF_EN] = {
+		.name = "tcsr_usb3_1_clkref_en",
+		.offset = 0x68,
+	},
+	[TCSR_USB4_1_CLKREF_EN] = {
+		.name = "tcsr_usb4_1_clkref_en",
+		.offset = 0x44,
+	},
+	[TCSR_USB4_2_CLKREF_EN] = {
+		.name = "tcsr_usb4_2_clkref_en",
+		.offset = 0x5c,
 	},
-};
-
-static struct clk_regmap *tcsr_cc_glymur_clocks[] = {
-	[TCSR_EDP_CLKREF_EN] = &tcsr_edp_clkref_en.clkr,
-	[TCSR_PCIE_1_CLKREF_EN] = &tcsr_pcie_1_clkref_en.clkr,
-	[TCSR_PCIE_2_CLKREF_EN] = &tcsr_pcie_2_clkref_en.clkr,
-	[TCSR_PCIE_3_CLKREF_EN] = &tcsr_pcie_3_clkref_en.clkr,
-	[TCSR_PCIE_4_CLKREF_EN] = &tcsr_pcie_4_clkref_en.clkr,
-	[TCSR_USB2_1_CLKREF_EN] = &tcsr_usb2_1_clkref_en.clkr,
-	[TCSR_USB2_2_CLKREF_EN] = &tcsr_usb2_2_clkref_en.clkr,
-	[TCSR_USB2_3_CLKREF_EN] = &tcsr_usb2_3_clkref_en.clkr,
-	[TCSR_USB2_4_CLKREF_EN] = &tcsr_usb2_4_clkref_en.clkr,
-	[TCSR_USB3_0_CLKREF_EN] = &tcsr_usb3_0_clkref_en.clkr,
-	[TCSR_USB3_1_CLKREF_EN] = &tcsr_usb3_1_clkref_en.clkr,
-	[TCSR_USB4_1_CLKREF_EN] = &tcsr_usb4_1_clkref_en.clkr,
-	[TCSR_USB4_2_CLKREF_EN] = &tcsr_usb4_2_clkref_en.clkr,
-};
-
-static const struct regmap_config tcsr_cc_glymur_regmap_config = {
-	.reg_bits = 32,
-	.reg_stride = 4,
-	.val_bits = 32,
-	.max_register = 0x94,
-	.fast_io = true,
-};
-
-static const struct qcom_cc_desc tcsr_cc_glymur_desc = {
-	.config = &tcsr_cc_glymur_regmap_config,
-	.clks = tcsr_cc_glymur_clocks,
-	.num_clks = ARRAY_SIZE(tcsr_cc_glymur_clocks),
 };
 
 static const struct of_device_id tcsr_cc_glymur_match_table[] = {
@@ -285,7 +119,9 @@ MODULE_DEVICE_TABLE(of, tcsr_cc_glymur_match_table);
 
 static int tcsr_cc_glymur_probe(struct platform_device *pdev)
 {
-	return qcom_cc_probe(pdev, &tcsr_cc_glymur_desc);
+	return qcom_clk_ref_probe(pdev, &tcsr_cc_glymur_regmap_config,
+				  tcsr_cc_glymur_clk_descs,
+				  ARRAY_SIZE(tcsr_cc_glymur_clk_descs));
 }
 
 static struct platform_driver tcsr_cc_glymur_driver = {

-- 
2.34.1

Re: [PATCH v4 3/7] clk: qcom: tcsrcc-glymur: Migrate tcsr_pcie_N_clkref_en to clk_ref common helper

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] TCSR_PCIE_4_CLKREF_EN incorrectly reuses the regulator mapping for PCIe 2, leaving its actual PHY unpowered.
--

commit d13bf7559fc4e2e26b752b7696e31f42885ef946
Author: Qiang Yu <qiang.yu@oss.qualcomm.com>

clk: qcom: tcsrcc-glymur: Migrate tcsr_pcie_N_clkref_en to clk_ref common helper

This commit replaces local clk_branch-based clkref definitions with
descriptor-based registration via qcom_clk_ref_probe(). It keeps the driver
focused on clock metadata and reuses common runtime logic.

> diff --git a/drivers/clk/qcom/tcsrcc-glymur.c b/drivers/clk/qcom/tcsrcc-glymur.c
> index 9c0edebcdbb12..e317003398d16 100644
> --- a/drivers/clk/qcom/tcsrcc-glymur.c
> +++ b/drivers/clk/qcom/tcsrcc-glymur.c
[ ... ]
> @@ -4,277 +4,111 @@
[ ... ]
> +static const struct qcom_clk_ref_desc tcsr_cc_glymur_clk_descs[] = {
[ ... ]
> +	[TCSR_PCIE_4_CLKREF_EN] = {
> +		.name = "tcsr_pcie_4_clkref_en",
> +		.offset = 0x58,
> +		.regulator_names = glymur_tcsr_tx1_rpt012_rx2_regulators,
> +		.num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt012_rx2_regulators),
>  	},

[Severity: High]
Does TCSR_PCIE_4_CLKREF_EN incorrectly reuse the regulator mapping for PCIe 2?

This array powers the rx2 and rpt2 regulators, which belong to the PCIe 2 PHY,
leaving the actual PCIe 4 PHY unpowered.

The device tree binding requires vdda-qrefrpt3-0p9-supply,
vdda-qrefrpt4-0p9-supply, and vdda-qrefrx4-0p9-supply for Glymur. 

Since these supplies are absent from the driver's defined arrays, should a
dedicated array be defined for the RX4 and RPT3/4 blocks instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527-tcsr_qref_0527-v4-0-ded83866c9d9@oss.qualcomm.com?part=3

Re: [PATCH v4 3/7] clk: qcom: tcsrcc-glymur: Migrate tcsr_pcie_N_clkref_en to clk_ref common helper

[Konrad Dybcio]

On 5/28/26 4:29 AM, Qiang Yu wrote:
> Replace local clk_branch-based clkref definitions with descriptor-based
> registration via qcom_clk_ref_probe().
> 
> This keeps the glymur driver focused on clock metadata and reuses common
> runtime logic for regulator handling, enable/disable sequencing, and OF
> provider wiring.
> 
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> ---

You can remove the of.h include. Apart from that:

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Also, attaching a diff to complete the regulator map. I'm fairly sure
these are correct, but it never hurts to triple-check.

You can add:

Co-developed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

if you squash them together. FYI e.g. the tertiary USB QMPPHY would
only start every 20 boots or so without this, running on pure luck..

Konrad

diff --git a/drivers/clk/qcom/tcsrcc-glymur.c b/drivers/clk/qcom/tcsrcc-glymur.c
index e317003398d1..eb4ee8ec9ad7 100644
--- a/drivers/clk/qcom/tcsrcc-glymur.c
+++ b/drivers/clk/qcom/tcsrcc-glymur.c
@@ -21,6 +21,14 @@ static const char * const glymur_tcsr_tx0_rx5_regulators[] = {
        "vdda-qreftx0-1p2",
 };
 
+static const char * const glymur_tcsr_tx1_rpt0_rx0_regulators[] = {
+       "vdda-refgen4-0p9",
+       "vdda-refgen4-1p2",
+       "vdda-qreftx1-0p9",
+       "vdda-qrefrpt0-0p9",
+       "vdda-qrefrx0-0p9",
+};
+
 static const char * const glymur_tcsr_tx1_rpt01_rx1_regulators[] = {
        "vdda-refgen4-0p9",
        "vdda-refgen4-1p2",
@@ -40,6 +48,15 @@ static const char * const glymur_tcsr_tx1_rpt012_rx2_regulators[] = {
        "vdda-qrefrx2-0p9",
 };
 
+static const char * const glymur_tcsr_tx1_rpt34_rx4_regulators[] = {
+       "vdda-refgen2-0p9",
+       "vdda-refgen2-1p2",
+       "vdda-qreftx1-0p9",
+       "vdda-qrefrpt3-0p9",
+       "vdda-qrefrpt4-0p9",
+       "vdda-qrefrx4-0p9",
+};
+
 static const struct regmap_config tcsr_cc_glymur_regmap_config = {
        .reg_bits = 32,
        .reg_stride = 4,
@@ -52,6 +69,8 @@ static const struct qcom_clk_ref_desc tcsr_cc_glymur_clk_descs[] = {
        [TCSR_EDP_CLKREF_EN] = {
                .name = "tcsr_edp_clkref_en",
                .offset = 0x60,
+               .regulator_names = glymur_tcsr_tx1_rpt0_rx0_regulators,
+               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt0_rx0_regulators),
        },
        [TCSR_PCIE_1_CLKREF_EN] = {
                .name = "tcsr_pcie_1_clkref_en",
@@ -80,34 +99,50 @@ static const struct qcom_clk_ref_desc tcsr_cc_glymur_clk_descs[] = {
        [TCSR_USB2_1_CLKREF_EN] = {
                .name = "tcsr_usb2_1_clkref_en",
                .offset = 0x6c,
+               .regulator_names = glymur_tcsr_tx1_rpt34_rx4_regulators,
+               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt34_rx4_regulators),
        },
        [TCSR_USB2_2_CLKREF_EN] = {
                .name = "tcsr_usb2_2_clkref_en",
                .offset = 0x70,
+               .regulator_names = glymur_tcsr_tx1_rpt01_rx1_regulators,
+               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt01_rx1_regulators),
        },
        [TCSR_USB2_3_CLKREF_EN] = {
                .name = "tcsr_usb2_3_clkref_en",
                .offset = 0x74,
+               .regulator_names = glymur_tcsr_tx1_rpt34_rx4_regulators,
+               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt34_rx4_regulators),
        },
        [TCSR_USB2_4_CLKREF_EN] = {
                .name = "tcsr_usb2_4_clkref_en",
                .offset = 0x88,
+               .regulator_names = glymur_tcsr_tx1_rpt34_rx4_regulators,
+               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt34_rx4_regulators),
        },
        [TCSR_USB3_0_CLKREF_EN] = {
                .name = "tcsr_usb3_0_clkref_en",
                .offset = 0x64,
+               .regulator_names = glymur_tcsr_tx1_rpt34_rx4_regulators,
+               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt34_rx4_regulators),
        },
        [TCSR_USB3_1_CLKREF_EN] = {
                .name = "tcsr_usb3_1_clkref_en",
                .offset = 0x68,
+               .regulator_names = glymur_tcsr_tx1_rpt34_rx4_regulators,
+               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt34_rx4_regulators),
        },
        [TCSR_USB4_1_CLKREF_EN] = {
                .name = "tcsr_usb4_1_clkref_en",
                .offset = 0x44,
+               .regulator_names = glymur_tcsr_tx0_rx5_regulators,
+               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx0_rx5_regulators),
        },
        [TCSR_USB4_2_CLKREF_EN] = {
                .name = "tcsr_usb4_2_clkref_en",
                .offset = 0x5c,
+               .regulator_names = glymur_tcsr_tx1_rpt01_rx1_regulators,
+               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt01_rx1_regulators),
        },
 };

Re: [PATCH v4 3/7] clk: qcom: tcsrcc-glymur: Migrate tcsr_pcie_N_clkref_en to clk_ref common helper

[Qiang Yu]

On Tue, Jun 09, 2026 at 03:02:27PM +0200, Konrad Dybcio wrote:
> On 5/28/26 4:29 AM, Qiang Yu wrote:
> > Replace local clk_branch-based clkref definitions with descriptor-based
> > registration via qcom_clk_ref_probe().
> > 
> > This keeps the glymur driver focused on clock metadata and reuses common
> > runtime logic for regulator handling, enable/disable sequencing, and OF
> > provider wiring.
> > 
> > Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> > ---
> 
> You can remove the of.h include. Apart from that:
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> Also, attaching a diff to complete the regulator map. I'm fairly sure
> these are correct, but it never hurts to triple-check.
> 
> You can add:
> 
> Co-developed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> if you squash them together. FYI e.g. the tertiary USB QMPPHY would
> only start every 20 boots or so without this, running on pure luck..
>

Okay, will sqash them and add above tags in next version.

- Qiang Yu
> Konrad
> 
> diff --git a/drivers/clk/qcom/tcsrcc-glymur.c b/drivers/clk/qcom/tcsrcc-glymur.c
> index e317003398d1..eb4ee8ec9ad7 100644
> --- a/drivers/clk/qcom/tcsrcc-glymur.c
> +++ b/drivers/clk/qcom/tcsrcc-glymur.c
> @@ -21,6 +21,14 @@ static const char * const glymur_tcsr_tx0_rx5_regulators[] = {
>         "vdda-qreftx0-1p2",
>  };
>  
> +static const char * const glymur_tcsr_tx1_rpt0_rx0_regulators[] = {
> +       "vdda-refgen4-0p9",
> +       "vdda-refgen4-1p2",
> +       "vdda-qreftx1-0p9",
> +       "vdda-qrefrpt0-0p9",
> +       "vdda-qrefrx0-0p9",
> +};
> +
>  static const char * const glymur_tcsr_tx1_rpt01_rx1_regulators[] = {
>         "vdda-refgen4-0p9",
>         "vdda-refgen4-1p2",
> @@ -40,6 +48,15 @@ static const char * const glymur_tcsr_tx1_rpt012_rx2_regulators[] = {
>         "vdda-qrefrx2-0p9",
>  };
>  
> +static const char * const glymur_tcsr_tx1_rpt34_rx4_regulators[] = {
> +       "vdda-refgen2-0p9",
> +       "vdda-refgen2-1p2",
> +       "vdda-qreftx1-0p9",
> +       "vdda-qrefrpt3-0p9",
> +       "vdda-qrefrpt4-0p9",
> +       "vdda-qrefrx4-0p9",
> +};
> +
>  static const struct regmap_config tcsr_cc_glymur_regmap_config = {
>         .reg_bits = 32,
>         .reg_stride = 4,
> @@ -52,6 +69,8 @@ static const struct qcom_clk_ref_desc tcsr_cc_glymur_clk_descs[] = {
>         [TCSR_EDP_CLKREF_EN] = {
>                 .name = "tcsr_edp_clkref_en",
>                 .offset = 0x60,
> +               .regulator_names = glymur_tcsr_tx1_rpt0_rx0_regulators,
> +               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt0_rx0_regulators),
>         },
>         [TCSR_PCIE_1_CLKREF_EN] = {
>                 .name = "tcsr_pcie_1_clkref_en",
> @@ -80,34 +99,50 @@ static const struct qcom_clk_ref_desc tcsr_cc_glymur_clk_descs[] = {
>         [TCSR_USB2_1_CLKREF_EN] = {
>                 .name = "tcsr_usb2_1_clkref_en",
>                 .offset = 0x6c,
> +               .regulator_names = glymur_tcsr_tx1_rpt34_rx4_regulators,
> +               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt34_rx4_regulators),
>         },
>         [TCSR_USB2_2_CLKREF_EN] = {
>                 .name = "tcsr_usb2_2_clkref_en",
>                 .offset = 0x70,
> +               .regulator_names = glymur_tcsr_tx1_rpt01_rx1_regulators,
> +               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt01_rx1_regulators),
>         },
>         [TCSR_USB2_3_CLKREF_EN] = {
>                 .name = "tcsr_usb2_3_clkref_en",
>                 .offset = 0x74,
> +               .regulator_names = glymur_tcsr_tx1_rpt34_rx4_regulators,
> +               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt34_rx4_regulators),
>         },
>         [TCSR_USB2_4_CLKREF_EN] = {
>                 .name = "tcsr_usb2_4_clkref_en",
>                 .offset = 0x88,
> +               .regulator_names = glymur_tcsr_tx1_rpt34_rx4_regulators,
> +               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt34_rx4_regulators),
>         },
>         [TCSR_USB3_0_CLKREF_EN] = {
>                 .name = "tcsr_usb3_0_clkref_en",
>                 .offset = 0x64,
> +               .regulator_names = glymur_tcsr_tx1_rpt34_rx4_regulators,
> +               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt34_rx4_regulators),
>         },
>         [TCSR_USB3_1_CLKREF_EN] = {
>                 .name = "tcsr_usb3_1_clkref_en",
>                 .offset = 0x68,
> +               .regulator_names = glymur_tcsr_tx1_rpt34_rx4_regulators,
> +               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt34_rx4_regulators),
>         },
>         [TCSR_USB4_1_CLKREF_EN] = {
>                 .name = "tcsr_usb4_1_clkref_en",
>                 .offset = 0x44,
> +               .regulator_names = glymur_tcsr_tx0_rx5_regulators,
> +               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx0_rx5_regulators),
>         },
>         [TCSR_USB4_2_CLKREF_EN] = {
>                 .name = "tcsr_usb4_2_clkref_en",
>                 .offset = 0x5c,
> +               .regulator_names = glymur_tcsr_tx1_rpt01_rx1_regulators,
> +               .num_regulators = ARRAY_SIZE(glymur_tcsr_tx1_rpt01_rx1_regulators),
>         },
>  };
> 

[PATCH v4 4/7] clk: qcom: tcsrcc-glymur: Add Mahua QREF regulator support

[Qiang Yu]

Mahua is based on Glymur but uses a different QREF topology, requiring
distinct regulator lists and clock descriptors for its PCIe clock
references.

Add mahua-specific regulator arrays and clk descriptor table, and use
match_data to select the correct descriptor table per compatible string at
probe time.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 drivers/clk/qcom/tcsrcc-glymur.c | 99 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 96 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/tcsrcc-glymur.c b/drivers/clk/qcom/tcsrcc-glymur.c
index e317003398d1..deca9b8794b4 100644
--- a/drivers/clk/qcom/tcsrcc-glymur.c
+++ b/drivers/clk/qcom/tcsrcc-glymur.c
@@ -13,6 +13,11 @@
 
 #include <dt-bindings/clock/qcom,glymur-tcsr.h>
 
+struct tcsrcc_glymur_data {
+	const struct qcom_clk_ref_desc *descs;
+	size_t num_descs;
+};
+
 static const char * const glymur_tcsr_tx0_rx5_regulators[] = {
 	"vdda-refgen3-0p9",
 	"vdda-refgen3-1p2",
@@ -40,6 +45,25 @@ static const char * const glymur_tcsr_tx1_rpt012_rx2_regulators[] = {
 	"vdda-qrefrx2-0p9",
 };
 
+static const char * const mahua_tcsr_tx1_rpt01_rx1_regulators[] = {
+	"vdda-refgen3-0p9",
+	"vdda-refgen3-1p2",
+	"vdda-qreftx1-0p9",
+	"vdda-qrefrpt0-0p9",
+	"vdda-qrefrpt1-0p9",
+	"vdda-qrefrx1-0p9",
+};
+
+static const char * const mahua_tcsr_tx1_rpt012_rx2_regulators[] = {
+	"vdda-refgen3-0p9",
+	"vdda-refgen3-1p2",
+	"vdda-qreftx1-0p9",
+	"vdda-qrefrpt0-0p9",
+	"vdda-qrefrpt1-0p9",
+	"vdda-qrefrpt2-0p9",
+	"vdda-qrefrx2-0p9",
+};
+
 static const struct regmap_config tcsr_cc_glymur_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -111,17 +135,86 @@ static const struct qcom_clk_ref_desc tcsr_cc_glymur_clk_descs[] = {
 	},
 };
 
+static const struct qcom_clk_ref_desc tcsr_cc_mahua_clk_descs[] = {
+	[TCSR_EDP_CLKREF_EN] = {
+		.name = "tcsr_edp_clkref_en",
+		.offset = 0x60,
+	},
+	[TCSR_PCIE_2_CLKREF_EN] = {
+		.name = "tcsr_pcie_2_clkref_en",
+		.offset = 0x4c,
+		.regulator_names = mahua_tcsr_tx1_rpt01_rx1_regulators,
+		.num_regulators = ARRAY_SIZE(mahua_tcsr_tx1_rpt01_rx1_regulators),
+	},
+	[TCSR_PCIE_3_CLKREF_EN] = {
+		.name = "tcsr_pcie_3_clkref_en",
+		.offset = 0x54,
+		.regulator_names = mahua_tcsr_tx1_rpt012_rx2_regulators,
+		.num_regulators = ARRAY_SIZE(mahua_tcsr_tx1_rpt012_rx2_regulators),
+	},
+	[TCSR_PCIE_4_CLKREF_EN] = {
+		.name = "tcsr_pcie_4_clkref_en",
+		.offset = 0x58,
+		.regulator_names = mahua_tcsr_tx1_rpt01_rx1_regulators,
+		.num_regulators = ARRAY_SIZE(mahua_tcsr_tx1_rpt01_rx1_regulators),
+	},
+	[TCSR_USB2_1_CLKREF_EN] = {
+		.name = "tcsr_usb2_1_clkref_en",
+		.offset = 0x6c,
+	},
+	[TCSR_USB2_2_CLKREF_EN] = {
+		.name = "tcsr_usb2_2_clkref_en",
+		.offset = 0x70,
+	},
+	[TCSR_USB2_3_CLKREF_EN] = {
+		.name = "tcsr_usb2_3_clkref_en",
+		.offset = 0x74,
+	},
+	[TCSR_USB2_4_CLKREF_EN] = {
+		.name = "tcsr_usb2_4_clkref_en",
+		.offset = 0x88,
+	},
+	[TCSR_USB3_0_CLKREF_EN] = {
+		.name = "tcsr_usb3_0_clkref_en",
+		.offset = 0x64,
+	},
+	[TCSR_USB3_1_CLKREF_EN] = {
+		.name = "tcsr_usb3_1_clkref_en",
+		.offset = 0x68,
+	},
+	[TCSR_USB4_1_CLKREF_EN] = {
+		.name = "tcsr_usb4_1_clkref_en",
+		.offset = 0x44,
+	},
+	[TCSR_USB4_2_CLKREF_EN] = {
+		.name = "tcsr_usb4_2_clkref_en",
+		.offset = 0x5c,
+	},
+};
+
+static const struct tcsrcc_glymur_data tcsr_cc_glymur_data = {
+	.descs     = tcsr_cc_glymur_clk_descs,
+	.num_descs = ARRAY_SIZE(tcsr_cc_glymur_clk_descs),
+};
+
+static const struct tcsrcc_glymur_data tcsr_cc_mahua_data = {
+	.descs     = tcsr_cc_mahua_clk_descs,
+	.num_descs = ARRAY_SIZE(tcsr_cc_mahua_clk_descs),
+};
+
 static const struct of_device_id tcsr_cc_glymur_match_table[] = {
-	{ .compatible = "qcom,glymur-tcsr" },
+	{ .compatible = "qcom,glymur-tcsr", .data = &tcsr_cc_glymur_data },
+	{ .compatible = "qcom,mahua-tcsr",  .data = &tcsr_cc_mahua_data  },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, tcsr_cc_glymur_match_table);
 
 static int tcsr_cc_glymur_probe(struct platform_device *pdev)
 {
+	const struct tcsrcc_glymur_data *data = device_get_match_data(&pdev->dev);
+
 	return qcom_clk_ref_probe(pdev, &tcsr_cc_glymur_regmap_config,
-				  tcsr_cc_glymur_clk_descs,
-				  ARRAY_SIZE(tcsr_cc_glymur_clk_descs));
+				  data->descs, data->num_descs);
 }
 
 static struct platform_driver tcsr_cc_glymur_driver = {

-- 
2.34.1

[PATCH v4 5/7] arm64: dts: qcom: glymur: Add QREF regulator supplies to TCSR

[Qiang Yu]

The TCSR clkref_en clocks gate the QREF block which provides reference
clocks to the PCIe PHYs. Wire up the LDO supplies required by the QREF
and refgen blocks on the CRD board.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/glymur-crd.dts | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/glymur-crd.dts b/arch/arm64/boot/dts/qcom/glymur-crd.dts
index c98dfb3941fa..be8497ef0da3 100644
--- a/arch/arm64/boot/dts/qcom/glymur-crd.dts
+++ b/arch/arm64/boot/dts/qcom/glymur-crd.dts
@@ -354,6 +354,26 @@ &usb_1 {
 	status = "okay";
 };
 
+&tcsr {
+	vdda-qrefrpt0-0p9-supply = <&vreg_l2f_e1_0p83>;
+	vdda-qrefrpt1-0p9-supply = <&vreg_l2f_e1_0p83>;
+	vdda-qrefrpt2-0p9-supply = <&vreg_l2f_e1_0p83>;
+	vdda-qrefrpt3-0p9-supply = <&vreg_l2h_e0_0p72>;
+	vdda-qrefrpt4-0p9-supply = <&vreg_l2h_e0_0p72>;
+	vdda-qrefrx0-0p9-supply = <&vreg_l2f_e1_0p83>;
+	vdda-qrefrx1-0p9-supply = <&vreg_l2f_e1_0p83>;
+	vdda-qrefrx2-0p9-supply = <&vreg_l2f_e1_0p83>;
+	vdda-qrefrx4-0p9-supply = <&vreg_l2h_e0_0p72>;
+	vdda-qrefrx5-0p9-supply = <&vreg_l3f_e0_0p72>;
+	vdda-qreftx0-0p9-supply = <&vreg_l3f_e0_0p72>;
+	vdda-qreftx0-1p2-supply = <&vreg_l4h_e0_1p2>;
+	vdda-qreftx1-0p9-supply = <&vreg_l1f_e1_0p82>;
+	vdda-refgen3-0p9-supply = <&vreg_l2f_e0_0p82>;
+	vdda-refgen3-1p2-supply = <&vreg_l4h_e0_1p2>;
+	vdda-refgen4-0p9-supply = <&vreg_l1f_e1_0p82>;
+	vdda-refgen4-1p2-supply = <&vreg_l4f_e1_1p08>;
+};
+
 &usb_1_dwc3_hs {
 	remote-endpoint = <&pmic_glink_hs_in1>;
 };

-- 
2.34.1

Re: [PATCH v4 5/7] arm64: dts: qcom: glymur: Add QREF regulator supplies to TCSR

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `&tcsr` node override is inserted out of alphabetical order in `arch/arm64/boot/dts/qcom/glymur-crd.dts`.
--

commit 9720c9e91ecfbc0e51190570b621f1da39f55b1b
Author: Qiang Yu <qiang.yu@oss.qualcomm.com>

arm64: dts: qcom: glymur: Add QREF regulator supplies to TCSR

This commit wires up the LDO supplies required by the QREF and refgen blocks
on the CRD board. The TCSR clkref_en clocks gate the QREF block which provides
reference clocks to the PCIe PHYs.

> diff --git a/arch/arm64/boot/dts/qcom/glymur-crd.dts b/arch/arm64/boot/dts/qcom/glymur-crd.dts
> index c98dfb3941fa3..be8497ef0da33 100644
> --- a/arch/arm64/boot/dts/qcom/glymur-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/glymur-crd.dts
> @@ -354,6 +354,26 @@ &usb_1 {
>  	status = "okay";
>  };
>  
> +&tcsr {

[Severity: Low]
This isn't a bug, but should the &tcsr node override be placed before the
&usb_1 node to maintain alphabetical sorting? Standard devicetree styling
guidelines for Qualcomm device trees usually require root-level node
overrides to be sorted alphabetically by their label to minimize merge
conflicts and maintain readability.

> +	vdda-qrefrpt0-0p9-supply = <&vreg_l2f_e1_0p83>;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527-tcsr_qref_0527-v4-0-ded83866c9d9@oss.qualcomm.com?part=5

[PATCH v4 6/7] arm64: dts: qcom: mahua: Add QREF regulator supplies to TCSR

[Qiang Yu]

Mahua has a different PCIe QREF topology from glymur. Override the TCSR
compatible to qcom,mahua-tcsr in mahua.dtsi, and wire up the required
LDO supplies for the PCIe clkref paths on the CRD board.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/mahua-crd.dts | 15 +++++++++++++++
 arch/arm64/boot/dts/qcom/mahua.dtsi    |  4 ++++
 2 files changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/mahua-crd.dts b/arch/arm64/boot/dts/qcom/mahua-crd.dts
index 9c8244e892dd..8b42f5174b31 100644
--- a/arch/arm64/boot/dts/qcom/mahua-crd.dts
+++ b/arch/arm64/boot/dts/qcom/mahua-crd.dts
@@ -19,3 +19,18 @@ / {
 	model = "Qualcomm Technologies, Inc. Mahua CRD";
 	compatible = "qcom,mahua-crd", "qcom,mahua";
 };
+
+&tcsr {
+	vdda-qrefrpt0-0p9-supply = <&vreg_l2f_e1_0p83>;
+	vdda-qrefrpt1-0p9-supply = <&vreg_l2f_e1_0p83>;
+	vdda-qrefrpt2-0p9-supply = <&vreg_l2f_e1_0p83>;
+	vdda-qrefrpt3-0p9-supply = <&vreg_l1f_e1_0p82>;
+	vdda-qrefrpt4-0p9-supply = <&vreg_l2h_e0_0p72>;
+	vdda-qrefrpt5-0p9-supply = <&vreg_l2h_e0_0p72>;
+	vdda-qrefrx1-0p9-supply = <&vreg_l2f_e1_0p83>;
+	vdda-qrefrx2-0p9-supply = <&vreg_l2f_e1_0p83>;
+	vdda-qrefrx3-0p9-supply = <&vreg_l2h_e0_0p72>;
+	vdda-qreftx1-0p9-supply = <&vreg_l1f_e1_0p82>;
+	vdda-refgen3-0p9-supply = <&vreg_l1f_e1_0p82>;
+	vdda-refgen3-1p2-supply = <&vreg_l4f_e1_1p08>;
+};
diff --git a/arch/arm64/boot/dts/qcom/mahua.dtsi b/arch/arm64/boot/dts/qcom/mahua.dtsi
index 22822b6b2e8b..eb45adc8a0a2 100644
--- a/arch/arm64/boot/dts/qcom/mahua.dtsi
+++ b/arch/arm64/boot/dts/qcom/mahua.dtsi
@@ -286,6 +286,10 @@ gpuss-4-critical {
 	};
 };
 
+&tcsr {
+	compatible = "qcom,mahua-tcsr", "syscon";
+};
+
 &tlmm {
 	compatible = "qcom,mahua-tlmm";
 };

-- 
2.34.1

[PATCH v4 7/7] arm64: dts: qcom: mahua: Switch pcie5_phy ref clock to RPMH_CXO_CLK

[Qiang Yu]

PCIe5 PHY on Mahua gets refclk from CXO0 pad directly, so no QREF
clkref_en voting is required. Override the clock list to use RPMH_CXO_CLK
directly instead.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/mahua.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/mahua.dtsi b/arch/arm64/boot/dts/qcom/mahua.dtsi
index eb45adc8a0a2..e6c059708912 100644
--- a/arch/arm64/boot/dts/qcom/mahua.dtsi
+++ b/arch/arm64/boot/dts/qcom/mahua.dtsi
@@ -115,6 +115,15 @@ &oobm_ss_noc {
 	compatible = "qcom,mahua-oobm-ss-noc", "qcom,glymur-oobm-ss-noc";
 };
 
+&pcie5_phy {
+	clocks = <&gcc GCC_PCIE_PHY_5_AUX_CLK>,
+		 <&gcc GCC_PCIE_5_CFG_AHB_CLK>,
+		 <&rpmhcc RPMH_CXO_CLK>,
+		 <&gcc GCC_PCIE_5_PHY_RCHNG_CLK>,
+		 <&gcc GCC_PCIE_5_PIPE_CLK>,
+		 <&gcc GCC_PCIE_5_PIPE_DIV2_CLK>;
+};
+
 &pcie_east_anoc {
 	compatible = "qcom,mahua-pcie-east-anoc", "qcom,glymur-pcie-east-anoc";
 };

-- 
2.34.1

Re: [PATCH v4 7/7] arm64: dts: qcom: mahua: Switch pcie5_phy ref clock to RPMH_CXO_CLK

[Konrad Dybcio]

On 5/28/26 4:29 AM, Qiang Yu wrote:
> PCIe5 PHY on Mahua gets refclk from CXO0 pad directly, so no QREF
> clkref_en voting is required. Override the clock list to use RPMH_CXO_CLK
> directly instead.

This is the last piece of the puzzle that this series is missing.
There's no QREF clkref_en, but there is a refgen that needs to be
powered. For PCIe5 on Mahua this would be L2F_E0 (0p9) and L4H_E0
(1p2).

I think the easiest (laziest?) solution would be to add dummy clocks
in the clkref driver and only toggle the required regulators. Another
option is to defer back to individual drivers (such as PCIe QMPPHY).

I kinda like the "one central node to drive power" approach, but I'm
not sure others agree, since it stretches truth just a tiny bit
(although not as much as one would think since there are *some*
controls for the transparent-to-the-OS hw pieces in these paths still
in TCSR).. Dmitry, Krzysztof, would you object to that?

Konrad

Re: [PATCH v4 7/7] arm64: dts: qcom: mahua: Switch pcie5_phy ref clock to RPMH_CXO_CLK

[Krzysztof Kozlowski]

On 09/06/2026 15:06, Konrad Dybcio wrote:
> On 5/28/26 4:29 AM, Qiang Yu wrote:
>> PCIe5 PHY on Mahua gets refclk from CXO0 pad directly, so no QREF
>> clkref_en voting is required. Override the clock list to use RPMH_CXO_CLK
>> directly instead.
> 
> This is the last piece of the puzzle that this series is missing.
> There's no QREF clkref_en, but there is a refgen that needs to be
> powered. For PCIe5 on Mahua this would be L2F_E0 (0p9) and L4H_E0
> (1p2).
> 
> I think the easiest (laziest?) solution would be to add dummy clocks
> in the clkref driver and only toggle the required regulators. Another
> option is to defer back to individual drivers (such as PCIe QMPPHY).
> 
> I kinda like the "one central node to drive power" approach, but I'm
> not sure others agree, since it stretches truth just a tiny bit
> (although not as much as one would think since there are *some*
> controls for the transparent-to-the-OS hw pieces in these paths still
> in TCSR).. Dmitry, Krzysztof, would you object to that?

Not sure what you ask here... the tcsr will get the refgen supplies and
that's all what is needed from DT.

Best regards,
Krzysztof

Re: [PATCH v4 7/7] arm64: dts: qcom: mahua: Switch pcie5_phy ref clock to RPMH_CXO_CLK

[Konrad Dybcio]

On 6/9/26 3:55 PM, Krzysztof Kozlowski wrote:
> On 09/06/2026 15:06, Konrad Dybcio wrote:
>> On 5/28/26 4:29 AM, Qiang Yu wrote:
>>> PCIe5 PHY on Mahua gets refclk from CXO0 pad directly, so no QREF
>>> clkref_en voting is required. Override the clock list to use RPMH_CXO_CLK
>>> directly instead.
>>
>> This is the last piece of the puzzle that this series is missing.
>> There's no QREF clkref_en, but there is a refgen that needs to be
>> powered. For PCIe5 on Mahua this would be L2F_E0 (0p9) and L4H_E0
>> (1p2).
>>
>> I think the easiest (laziest?) solution would be to add dummy clocks
>> in the clkref driver and only toggle the required regulators. Another
>> option is to defer back to individual drivers (such as PCIe QMPPHY).
>>
>> I kinda like the "one central node to drive power" approach, but I'm
>> not sure others agree, since it stretches truth just a tiny bit
>> (although not as much as one would think since there are *some*
>> controls for the transparent-to-the-OS hw pieces in these paths still
>> in TCSR).. Dmitry, Krzysztof, would you object to that?
> 
> Not sure what you ask here... the tcsr will get the refgen supplies and
> that's all what is needed from DT.

What I'm saying is that there are always-on clock branches (no register
to control them) which go through controllable repeaters (with a reg in
tcsr, although we never control them since the reset state is OK)

Both the repeaters and the associated refgens need power supplies. My
suggestion was to take care of that by adding "fake" clocks. Currently,
we have e.g.:

&usb_0_qmpphy {
        vdda-phy-supply = <&vreg_l4h_e0_1p2>;
        vdda-pll-supply = <&vreg_l3f_e0_0p72>;
        refgen-supply = <&vreg_l2f_e0_0p82>; // the other 1.2v refgen is missing

        status = "okay";
};

The refgen supply is deferred to the QMPPHY node itself, and the
repeaters on the way are ignored. The "ref" clock points to RPMH_XO.

What I'm suggesting is that we add "fake" (i.e. uncontrollable) TCSR
clocks, such as the inexistent TCSR_USB4_0_CLKREF_EN, to which we can
link the required regulators, both for the repeaters and the refgen
source

Konrad

Re: [PATCH v4 7/7] arm64: dts: qcom: mahua: Switch pcie5_phy ref clock to RPMH_CXO_CLK

[Qiang Yu]

On Tue, Jun 09, 2026 at 03:06:02PM +0200, Konrad Dybcio wrote:
> On 5/28/26 4:29 AM, Qiang Yu wrote:
> > PCIe5 PHY on Mahua gets refclk from CXO0 pad directly, so no QREF
> > clkref_en voting is required. Override the clock list to use RPMH_CXO_CLK
> > directly instead.
> 
> This is the last piece of the puzzle that this series is missing.
> There's no QREF clkref_en, but there is a refgen that needs to be
> powered. For PCIe5 on Mahua this would be L2F_E0 (0p9) and L4H_E0
> (1p2).
> 
> I think the easiest (laziest?) solution would be to add dummy clocks
> in the clkref driver and only toggle the required regulators. Another
> option is to defer back to individual drivers (such as PCIe QMPPHY).
> 
> I kinda like the "one central node to drive power" approach, but I'm
> not sure others agree, since it stretches truth just a tiny bit
> (although not as much as one would think since there are *some*
> controls for the transparent-to-the-OS hw pieces in these paths still
> in TCSR).. Dmitry, Krzysztof, would you object to that?
>

PCIe5 PHY on Mahua does not use QREF at all, so there is no refgen for
QREF either. The refgen supplies you mentioned are for the PCIe5 PHY
itself, not for QREF. For other PHYs that do use QREF, there are two
refgens: one for QREF (voted here in the TCSR clkref driver) and one for
the PHY (which should be voted in the PHY driver).

- Qiang Yu
> Konrad

Re: [PATCH v4 7/7] arm64: dts: qcom: mahua: Switch pcie5_phy ref clock to RPMH_CXO_CLK

[Konrad Dybcio]

On 6/15/26 10:51 AM, Qiang Yu wrote:
> On Tue, Jun 09, 2026 at 03:06:02PM +0200, Konrad Dybcio wrote:
>> On 5/28/26 4:29 AM, Qiang Yu wrote:
>>> PCIe5 PHY on Mahua gets refclk from CXO0 pad directly, so no QREF
>>> clkref_en voting is required. Override the clock list to use RPMH_CXO_CLK
>>> directly instead.
>>
>> This is the last piece of the puzzle that this series is missing.
>> There's no QREF clkref_en, but there is a refgen that needs to be
>> powered. For PCIe5 on Mahua this would be L2F_E0 (0p9) and L4H_E0
>> (1p2).
>>
>> I think the easiest (laziest?) solution would be to add dummy clocks
>> in the clkref driver and only toggle the required regulators. Another
>> option is to defer back to individual drivers (such as PCIe QMPPHY).
>>
>> I kinda like the "one central node to drive power" approach, but I'm
>> not sure others agree, since it stretches truth just a tiny bit
>> (although not as much as one would think since there are *some*
>> controls for the transparent-to-the-OS hw pieces in these paths still
>> in TCSR).. Dmitry, Krzysztof, would you object to that?
>>
> 
> PCIe5 PHY on Mahua does not use QREF at all, so there is no refgen for
> QREF either. The refgen supplies you mentioned are for the PCIe5 PHY
> itself, not for QREF. For other PHYs that do use QREF, there are two
> refgens: one for QREF (voted here in the TCSR clkref driver) and one for
> the PHY (which should be voted in the PHY driver).

Okay, so in this case we have the refgen regulator hardwired into the
PHY block and we just consume it from the PHY node&driver, am I following
correctly?

Konrad

Re: [PATCH v4 7/7] arm64: dts: qcom: mahua: Switch pcie5_phy ref clock to RPMH_CXO_CLK

[Qiang Yu]

On Wed, Jun 17, 2026 at 02:11:33PM +0200, Konrad Dybcio wrote:
> On 6/15/26 10:51 AM, Qiang Yu wrote:
> > On Tue, Jun 09, 2026 at 03:06:02PM +0200, Konrad Dybcio wrote:
> >> On 5/28/26 4:29 AM, Qiang Yu wrote:
> >>> PCIe5 PHY on Mahua gets refclk from CXO0 pad directly, so no QREF
> >>> clkref_en voting is required. Override the clock list to use RPMH_CXO_CLK
> >>> directly instead.
> >>
> >> This is the last piece of the puzzle that this series is missing.
> >> There's no QREF clkref_en, but there is a refgen that needs to be
> >> powered. For PCIe5 on Mahua this would be L2F_E0 (0p9) and L4H_E0
> >> (1p2).
> >>
> >> I think the easiest (laziest?) solution would be to add dummy clocks
> >> in the clkref driver and only toggle the required regulators. Another
> >> option is to defer back to individual drivers (such as PCIe QMPPHY).
> >>
> >> I kinda like the "one central node to drive power" approach, but I'm
> >> not sure others agree, since it stretches truth just a tiny bit
> >> (although not as much as one would think since there are *some*
> >> controls for the transparent-to-the-OS hw pieces in these paths still
> >> in TCSR).. Dmitry, Krzysztof, would you object to that?
> >>
> > 
> > PCIe5 PHY on Mahua does not use QREF at all, so there is no refgen for
> > QREF either. The refgen supplies you mentioned are for the PCIe5 PHY
> > itself, not for QREF. For other PHYs that do use QREF, there are two
> > refgens: one for QREF (voted here in the TCSR clkref driver) and one for
> > the PHY (which should be voted in the PHY driver).
> 
> Okay, so in this case we have the refgen regulator hardwired into the
> PHY block and we just consume it from the PHY node&driver, am I following
> correctly?
>

The refgen regulators are not hardwired into the QREF or PHY blocks
directly. They power the REFGEN block, which provides the reference
voltage to QREF and PHY.

- Qiang Yu