[PATCH 0/2] Add support for the REFGEN in the IPQ9650 SoC

[PATCH 0/2] Add support for the REFGEN in the IPQ9650 SoC

[Kathiravan Thirumoorthy]

IPQ9650 SoC has 2 REFGEN blocks providing the reference current to
the PCIe and USB, UNIPHY PHYs. For the other SoCs, clocks for this block
is enabled on power up but that's not the case for IPQ9650 and we have
to explicitly enable those clocks.

Document the same and add support for it.

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
Kathiravan Thirumoorthy (2):
      regulator: dt-bindings: qcom,sdm845-refgen-regulator: Document IPQ9650
      regulator: qcom-refgen: add support for the IPQ9650 SoC

 .../regulator/qcom,sdm845-refgen-regulator.yaml    | 21 +++++
 drivers/regulator/qcom-refgen-regulator.c          | 94 +++++++++++++++++++++-
 2 files changed, 111 insertions(+), 4 deletions(-)
---
base-commit: 08484c504b55a98bd100527fbe10a3caf55ff3ff
change-id: 20260520-ipq9650_refgen-196b570d8bc0

Best regards,
--  
Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>

[PATCH 1/2] regulator: dt-bindings: qcom,sdm845-refgen-regulator: Document IPQ9650

[Kathiravan Thirumoorthy]

IPQ9650 has two REFGEN blocks which provide reference current to the PCIe,
USB and UNIPHY PHYs. Unlike other supported platforms, IPQ9650 requires the
REFGEN clocks to be enabled explicitly.

Document the IPQ9650 compatible and the required clocks for it.

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
 .../regulator/qcom,sdm845-refgen-regulator.yaml     | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/qcom,sdm845-refgen-regulator.yaml b/Documentation/devicetree/bindings/regulator/qcom,sdm845-refgen-regulator.yaml
index 40f9223d4c27..2686569ca060 100644
--- a/Documentation/devicetree/bindings/regulator/qcom,sdm845-refgen-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/qcom,sdm845-refgen-regulator.yaml
@@ -16,6 +16,16 @@ description:
 allOf:
   - $ref: regulator.yaml#
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,ipq9650-refgen-regulator
+    then:
+      required:
+        - clocks
+        - clock-names
+
 properties:
   compatible:
     oneOf:
@@ -29,6 +39,7 @@ properties:
 
       - items:
           - enum:
+              - qcom,ipq9650-refgen-regulator
               - qcom,qcs8300-refgen-regulator
               - qcom,sa8775p-refgen-regulator
               - qcom,sc7280-refgen-regulator
@@ -45,6 +56,16 @@ properties:
   reg:
     maxItems: 1
 
+  clocks:
+    items:
+      - description: Core reference clock
+      - description: AHB interface clock
+
+  clock-names:
+    items:
+      - const: core
+      - const: hclk
+
 required:
   - compatible
   - reg

-- 
2.34.1

[PATCH 2/2] regulator: qcom-refgen: add support for the IPQ9650 SoC

[Kathiravan Thirumoorthy]

IPQ9650 SoC has 2 REFGEN blocks providing the reference current to the
PCIe and USB, UNIPHY PHYs. For the other SoCs, clocks for this block is
enabled on power up but that's not the case for IPQ9650 and we have to
enable those clocks explicitly to bring up the PHYs properly.

As per the design team, REFGEN block provides the reference current.
Hence marked the regulator type as REGULATOR_CURRENT.

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
 drivers/regulator/qcom-refgen-regulator.c | 94 +++++++++++++++++++++++++++++--
 1 file changed, 90 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/qcom-refgen-regulator.c b/drivers/regulator/qcom-refgen-regulator.c
index 299ac3c8c3bc..2858792acba8 100644
--- a/drivers/regulator/qcom-refgen-regulator.c
+++ b/drivers/regulator/qcom-refgen-regulator.c
@@ -3,6 +3,7 @@
 // Copyright (c) 2023, Linaro Limited
 
 #include <linux/bitfield.h>
+#include <linux/clk.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
@@ -10,6 +11,7 @@
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
 #include <linux/regulator/of_regulator.h>
+#include <linux/slab.h>
 
 #define REFGEN_REG_BIAS_EN		0x08
 #define REFGEN_BIAS_EN_MASK		GENMASK(2, 0)
@@ -25,6 +27,17 @@
 #define REFGEN_PWRDWN_CTRL5_MASK	BIT(0)
  #define REFGEN_PWRDWN_CTRL5_ENABLE	0x1
 
+struct qcom_refgen_regulator_data {
+	const struct regulator_desc *rdesc;
+	bool has_clocks;
+};
+
+struct qcom_refgen_drvdata {
+	struct clk_bulk_data *clks;
+	int num_clks;
+	unsigned int enable_count;
+};
+
 static int qcom_sdm845_refgen_enable(struct regulator_dev *rdev)
 {
 	regmap_update_bits(rdev->regmap, REFGEN_REG_BG_CTRL, REFGEN_BG_CTRL_MASK,
@@ -62,6 +75,49 @@ static int qcom_sdm845_refgen_is_enabled(struct regulator_dev *rdev)
 	return 1;
 }
 
+static int qcom_ipq9650_refgen_enable(struct regulator_dev *rdev)
+{
+	struct qcom_refgen_drvdata *drvdata = rdev_get_drvdata(rdev);
+	int ret;
+
+	ret = clk_bulk_prepare_enable(drvdata->num_clks, drvdata->clks);
+	if (ret)
+		return ret;
+
+	drvdata->enable_count++;
+
+	return 0;
+}
+
+static int qcom_ipq9650_refgen_disable(struct regulator_dev *rdev)
+{
+	struct qcom_refgen_drvdata *drvdata = rdev_get_drvdata(rdev);
+
+	clk_bulk_disable_unprepare(drvdata->num_clks, drvdata->clks);
+	drvdata->enable_count--;
+
+	return 0;
+}
+
+static int qcom_ipq9650_refgen_is_enabled(struct regulator_dev *rdev)
+{
+	struct qcom_refgen_drvdata *drvdata = rdev_get_drvdata(rdev);
+
+	return drvdata->enable_count > 0;
+}
+
+static const struct regulator_desc ipq9650_refgen_desc = {
+	.enable_time = 5,
+	.name = "refgen",
+	.owner = THIS_MODULE,
+	.type = REGULATOR_CURRENT,
+	.ops = &(const struct regulator_ops) {
+		.enable		= qcom_ipq9650_refgen_enable,
+		.disable	= qcom_ipq9650_refgen_disable,
+		.is_enabled	= qcom_ipq9650_refgen_is_enabled,
+	},
+};
+
 static const struct regulator_desc sdm845_refgen_desc = {
 	.enable_time = 5,
 	.name = "refgen",
@@ -90,6 +146,19 @@ static const struct regulator_desc sm8250_refgen_desc = {
 	},
 };
 
+static const struct qcom_refgen_regulator_data ipq9650_data = {
+	.rdesc = &ipq9650_refgen_desc,
+	.has_clocks = true,
+};
+
+static const struct qcom_refgen_regulator_data sdm845_data = {
+	.rdesc = &sdm845_refgen_desc,
+};
+
+static const struct qcom_refgen_regulator_data sm8250_data = {
+	.rdesc = &sm8250_refgen_desc,
+};
+
 static const struct regmap_config qcom_refgen_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -98,6 +167,8 @@ static const struct regmap_config qcom_refgen_regmap_config = {
 
 static int qcom_refgen_probe(struct platform_device *pdev)
 {
+	const struct qcom_refgen_regulator_data *data;
+	struct qcom_refgen_drvdata *drvdata = NULL;
 	struct regulator_init_data *init_data;
 	struct regulator_config config = {};
 	const struct regulator_desc *rdesc;
@@ -106,10 +177,23 @@ static int qcom_refgen_probe(struct platform_device *pdev)
 	struct regmap *regmap;
 	void __iomem *base;
 
-	rdesc = of_device_get_match_data(dev);
-	if (!rdesc)
+	data = of_device_get_match_data(dev);
+	if (!data)
 		return -ENODATA;
 
+	if (data->has_clocks) {
+		drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+		if (!drvdata)
+			return -ENOMEM;
+
+		drvdata->num_clks = devm_clk_bulk_get_all(dev, &drvdata->clks);
+		if (drvdata->num_clks < 0)
+			return dev_err_probe(dev, drvdata->num_clks,
+					     "failed to get clocks\n");
+	}
+
+	rdesc = data->rdesc;
+
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
@@ -126,6 +210,7 @@ static int qcom_refgen_probe(struct platform_device *pdev)
 	config.init_data = init_data;
 	config.of_node = dev->of_node;
 	config.regmap = regmap;
+	config.driver_data = drvdata;
 
 	rdev = devm_regulator_register(dev, rdesc, &config);
 	if (IS_ERR(rdev))
@@ -135,8 +220,9 @@ static int qcom_refgen_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_refgen_match_table[] = {
-	{ .compatible = "qcom,sdm845-refgen-regulator", .data = &sdm845_refgen_desc },
-	{ .compatible = "qcom,sm8250-refgen-regulator", .data = &sm8250_refgen_desc },
+	{ .compatible = "qcom,ipq9650-refgen-regulator", .data = &ipq9650_data },
+	{ .compatible = "qcom,sdm845-refgen-regulator", .data = &sdm845_data },
+	{ .compatible = "qcom,sm8250-refgen-regulator", .data = &sm8250_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, qcom_refgen_match_table);

-- 
2.34.1

Re: [PATCH 1/2] regulator: dt-bindings: qcom,sdm845-refgen-regulator: Document IPQ9650

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `qcom,ipq9650-refgen-regulator` compatible incorrectly defines `qcom,sm8250-refgen-regulator` as a fallback, falsely asserting backwards compatibility and creating a crash risk for older kernels.
--

commit 53a9f2cc53f31d4f470e020ea803a060c384000b
Author: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>

regulator: dt-bindings: qcom,sdm845-refgen-regulator: Document IPQ9650

This commit adds the qcom,ipq9650-refgen-regulator compatible and documents
its requirement for explicit core and AHB clocks.

> diff --git a/Documentation/devicetree/bindings/regulator/qcom,sdm845-refgen-regulator.yaml b/Documentation/devicetree/bindings/regulator/qcom,sdm845-refgen-regulator.yaml
> index 40f9223d4c272..2686569ca0606 100644
> --- a/Documentation/devicetree/bindings/regulator/qcom,sdm845-refgen-regulator.yaml
> +++ b/Documentation/devicetree/bindings/regulator/qcom,sdm845-refgen-regulator.yaml

[ ... ]

> @@ -29,6 +39,7 @@ properties:
>  
>        - items:
>            - enum:
> +              - qcom,ipq9650-refgen-regulator
>                - qcom,qcs8300-refgen-regulator
>                - qcom,sa8775p-refgen-regulator
>                - qcom,sc7280-refgen-regulator

[Severity: Medium]
The commit message states IPQ9650 explicitly requires its AHB and core clocks
to be enabled. However, adding the compatible string to this specific enum
block defines qcom,sm8250-refgen-regulator as its fallback.

The qcom,sm8250-refgen-regulator fallback driver does not handle clock
management. If an older kernel binds to this device using the fallback, it
will attempt to access the peripheral's MMIO registers without enabling the
AHB interface clock.

Since accessing unclocked AHB registers typically triggers a synchronous
external abort on these platforms, does this fallback definition create a fatal
system crash risk?

Should qcom,ipq9650-refgen-regulator be defined as a standalone compatible
string without a fallback instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602-ipq9650_refgen-v1-0-55e2afa5ff64@oss.qualcomm.com?part=1

Re: [PATCH 2/2] regulator: qcom-refgen: add support for the IPQ9650 SoC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Silent failure when DT clock properties are missing for a device requiring clocks.
--

commit 526ede5fae05c2893929b211a8654512286f1196
Author: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>

regulator: qcom-refgen: add support for the IPQ9650 SoC

This commit adds support for the IPQ9650 SoC to the qcom-refgen regulator
driver. Unlike other SoCs, the IPQ9650 requires its clocks to be explicitly
enabled to properly bring up the dependent PHYs. The regulator is also correctly
marked as REGULATOR_CURRENT based on the hardware design.

> diff --git a/drivers/regulator/qcom-refgen-regulator.c b/drivers/regulator/qcom-refgen-regulator.c
> --- a/drivers/regulator/qcom-refgen-regulator.c
> +++ b/drivers/regulator/qcom-refgen-regulator.c
[ ... ]
> @@ -106,10 +177,23 @@ static int qcom_refgen_probe(struct platform_device *pdev)
>  	struct regmap *regmap;
>  	void __iomem *base;
>  
> -	rdesc = of_device_get_match_data(dev);
> -	if (!rdesc)
> +	data = of_device_get_match_data(dev);
> +	if (!data)
>  		return -ENODATA;
>  
> +	if (data->has_clocks) {
> +		drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +		if (!drvdata)
> +			return -ENOMEM;
> +
> +		drvdata->num_clks = devm_clk_bulk_get_all(dev, &drvdata->clks);
> +		if (drvdata->num_clks < 0)

[Severity: Medium]
Should this check also handle the case where devm_clk_bulk_get_all() returns 0?

If the device tree is missing the expected clock properties,
devm_clk_bulk_get_all() will return 0. The current check allows 0 to pass
as a success, which could allow the probe to silently succeed without having
any clocks available.

Since the IPQ9650 explicitly requires these clocks to bring up the PHYs
properly, could this cause a silent failure downstream when the unpowered
REFGEN block is accessed?

> +			return dev_err_probe(dev, drvdata->num_clks,
> +					     "failed to get clocks\n");
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602-ipq9650_refgen-v1-0-55e2afa5ff64@oss.qualcomm.com?part=2

Re: [PATCH 2/2] regulator: qcom-refgen: add support for the IPQ9650 SoC

[Kathiravan Thirumoorthy]

On 6/2/2026 3:08 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Silent failure when DT clock properties are missing for a device requiring clocks.
> --
>
> commit 526ede5fae05c2893929b211a8654512286f1196
> Author: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
>
> regulator: qcom-refgen: add support for the IPQ9650 SoC
>
> This commit adds support for the IPQ9650 SoC to the qcom-refgen regulator
> driver. Unlike other SoCs, the IPQ9650 requires its clocks to be explicitly
> enabled to properly bring up the dependent PHYs. The regulator is also correctly
> marked as REGULATOR_CURRENT based on the hardware design.
>
>> diff --git a/drivers/regulator/qcom-refgen-regulator.c b/drivers/regulator/qcom-refgen-regulator.c
>> --- a/drivers/regulator/qcom-refgen-regulator.c
>> +++ b/drivers/regulator/qcom-refgen-regulator.c
> [ ... ]
>> @@ -106,10 +177,23 @@ static int qcom_refgen_probe(struct platform_device *pdev)
>>   	struct regmap *regmap;
>>   	void __iomem *base;
>>   
>> -	rdesc = of_device_get_match_data(dev);
>> -	if (!rdesc)
>> +	data = of_device_get_match_data(dev);
>> +	if (!data)
>>   		return -ENODATA;
>>   
>> +	if (data->has_clocks) {
>> +		drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>> +		if (!drvdata)
>> +			return -ENOMEM;
>> +
>> +		drvdata->num_clks = devm_clk_bulk_get_all(dev, &drvdata->clks);
>> +		if (drvdata->num_clks < 0)
> [Severity: Medium]
> Should this check also handle the case where devm_clk_bulk_get_all() returns 0?
>
> If the device tree is missing the expected clock properties,
> devm_clk_bulk_get_all() will return 0. The current check allows 0 to pass
> as a success, which could allow the probe to silently succeed without having
> any clocks available.

This validation should be performed by the DT schema and the drivers 
should just get all the clocks from DT. So, I think we can skip this 
report. Please let me know if I'm wrong.

>
> Since the IPQ9650 explicitly requires these clocks to bring up the PHYs
> properly, could this cause a silent failure downstream when the unpowered
> REFGEN block is accessed?
>
>> +			return dev_err_probe(dev, drvdata->num_clks,
>> +					     "failed to get clocks\n");
>> +	}
>> +

Re: [PATCH 2/2] regulator: qcom-refgen: add support for the IPQ9650 SoC

[Dmitry Baryshkov]

On Tue, Jun 02, 2026 at 02:52:00PM +0530, Kathiravan Thirumoorthy wrote:
> IPQ9650 SoC has 2 REFGEN blocks providing the reference current to the
> PCIe and USB, UNIPHY PHYs. For the other SoCs, clocks for this block is
> enabled on power up but that's not the case for IPQ9650 and we have to
> enable those clocks explicitly to bring up the PHYs properly.
> 
> As per the design team, REFGEN block provides the reference current.
> Hence marked the regulator type as REGULATOR_CURRENT.
> 
> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
> ---
>  drivers/regulator/qcom-refgen-regulator.c | 94 +++++++++++++++++++++++++++++--
>  1 file changed, 90 insertions(+), 4 deletions(-)
> 
> @@ -62,6 +75,49 @@ static int qcom_sdm845_refgen_is_enabled(struct regulator_dev *rdev)
>  	return 1;
>  }
>  
> +static int qcom_ipq9650_refgen_enable(struct regulator_dev *rdev)
> +{
> +	struct qcom_refgen_drvdata *drvdata = rdev_get_drvdata(rdev);
> +	int ret;
> +
> +	ret = clk_bulk_prepare_enable(drvdata->num_clks, drvdata->clks);
> +	if (ret)
> +		return ret;
> +
> +	drvdata->enable_count++;

I think, a regulator enable() is called only once. Is there a point in
having enable_count as int?

> +
> +	return 0;
> +}
> +
> +static int qcom_ipq9650_refgen_disable(struct regulator_dev *rdev)
> +{
> +	struct qcom_refgen_drvdata *drvdata = rdev_get_drvdata(rdev);
> +
> +	clk_bulk_disable_unprepare(drvdata->num_clks, drvdata->clks);
> +	drvdata->enable_count--;
> +
> +	return 0;
> +}
> +
> +static int qcom_ipq9650_refgen_is_enabled(struct regulator_dev *rdev)
> +{
> +	struct qcom_refgen_drvdata *drvdata = rdev_get_drvdata(rdev);
> +
> +	return drvdata->enable_count > 0;
> +}

Linux knows if it had enabled the regulator. I think the usual case for
the is_enabled is to be able to read the hardware state. What is the
point of having this callback?

> +
> +static const struct regulator_desc ipq9650_refgen_desc = {
> +	.enable_time = 5,
> +	.name = "refgen",
> +	.owner = THIS_MODULE,
> +	.type = REGULATOR_CURRENT,
> +	.ops = &(const struct regulator_ops) {
> +		.enable		= qcom_ipq9650_refgen_enable,
> +		.disable	= qcom_ipq9650_refgen_disable,
> +		.is_enabled	= qcom_ipq9650_refgen_is_enabled,
> +	},
> +};
> +
>  static const struct regulator_desc sdm845_refgen_desc = {
>  	.enable_time = 5,
>  	.name = "refgen",

-- 
With best wishes
Dmitry

Re: [PATCH 2/2] regulator: qcom-refgen: add support for the IPQ9650 SoC

[Kathiravan Thirumoorthy]

On 6/3/2026 6:47 PM, Dmitry Baryshkov wrote:
> On Tue, Jun 02, 2026 at 02:52:00PM +0530, Kathiravan Thirumoorthy wrote:
>> IPQ9650 SoC has 2 REFGEN blocks providing the reference current to the
>> PCIe and USB, UNIPHY PHYs. For the other SoCs, clocks for this block is
>> enabled on power up but that's not the case for IPQ9650 and we have to
>> enable those clocks explicitly to bring up the PHYs properly.
>>
>> As per the design team, REFGEN block provides the reference current.
>> Hence marked the regulator type as REGULATOR_CURRENT.
>>
>> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
>> ---
>>   drivers/regulator/qcom-refgen-regulator.c | 94 +++++++++++++++++++++++++++++--
>>   1 file changed, 90 insertions(+), 4 deletions(-)
>>
>> @@ -62,6 +75,49 @@ static int qcom_sdm845_refgen_is_enabled(struct regulator_dev *rdev)
>>   	return 1;
>>   }
>>   
>> +static int qcom_ipq9650_refgen_enable(struct regulator_dev *rdev)
>> +{
>> +	struct qcom_refgen_drvdata *drvdata = rdev_get_drvdata(rdev);
>> +	int ret;
>> +
>> +	ret = clk_bulk_prepare_enable(drvdata->num_clks, drvdata->clks);
>> +	if (ret)
>> +		return ret;
>> +
>> +	drvdata->enable_count++;
> I think, a regulator enable() is called only once. Is there a point in
> having enable_count as int?

Ack. Let me change it to boolean type.

>
>> +
>> +	return 0;
>> +}
>> +
>> +static int qcom_ipq9650_refgen_disable(struct regulator_dev *rdev)
>> +{
>> +	struct qcom_refgen_drvdata *drvdata = rdev_get_drvdata(rdev);
>> +
>> +	clk_bulk_disable_unprepare(drvdata->num_clks, drvdata->clks);
>> +	drvdata->enable_count--;
>> +
>> +	return 0;
>> +}
>> +
>> +static int qcom_ipq9650_refgen_is_enabled(struct regulator_dev *rdev)
>> +{
>> +	struct qcom_refgen_drvdata *drvdata = rdev_get_drvdata(rdev);
>> +
>> +	return drvdata->enable_count > 0;
>> +}
> Linux knows if it had enabled the regulator. I think the usual case for
> the is_enabled is to be able to read the hardware state. What is the
> point of having this callback?

Without the is_enabled(), regulator core assumes that regulator is 
enabled and always-on and is_enabled() is not called. Hence, it is 
needed in this case.

3458 static int _regulator_is_enabled(struct regulator_dev *rdev)
3459 {
3460         /* A GPIO control always takes precedence */
3461         if (rdev->ena_pin)
3462                 return rdev->ena_gpio_state;
3463
3464         /* If we don't know then assume that the regulator is 
always on */
3465         if (!rdev->desc->ops->is_enabled)
3466                 return 1;
3467
3468         return rdev->desc->ops->is_enabled(rdev);
3469 }

>
>> +
>> +static const struct regulator_desc ipq9650_refgen_desc = {
>> +	.enable_time = 5,
>> +	.name = "refgen",
>> +	.owner = THIS_MODULE,
>> +	.type = REGULATOR_CURRENT,
>> +	.ops = &(const struct regulator_ops) {
>> +		.enable		= qcom_ipq9650_refgen_enable,
>> +		.disable	= qcom_ipq9650_refgen_disable,
>> +		.is_enabled	= qcom_ipq9650_refgen_is_enabled,
>> +	},
>> +};
>> +
>>   static const struct regulator_desc sdm845_refgen_desc = {
>>   	.enable_time = 5,
>>   	.name = "refgen",

Re: [PATCH 1/2] regulator: dt-bindings: qcom,sdm845-refgen-regulator: Document IPQ9650

[Krzysztof Kozlowski]

On 02/06/2026 11:21, Kathiravan Thirumoorthy wrote:
> IPQ9650 has two REFGEN blocks which provide reference current to the PCIe,
> USB and UNIPHY PHYs. Unlike other supported platforms, IPQ9650 requires the
> REFGEN clocks to be enabled explicitly.
> 
> Document the IPQ9650 compatible and the required clocks for it.
> 
> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
> ---
>  .../regulator/qcom,sdm845-refgen-regulator.yaml     | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/qcom,sdm845-refgen-regulator.yaml b/Documentation/devicetree/bindings/regulator/qcom,sdm845-refgen-regulator.yaml
> index 40f9223d4c27..2686569ca060 100644
> --- a/Documentation/devicetree/bindings/regulator/qcom,sdm845-refgen-regulator.yaml
> +++ b/Documentation/devicetree/bindings/regulator/qcom,sdm845-refgen-regulator.yaml
> @@ -16,6 +16,16 @@ description:
>  allOf:
>    - $ref: regulator.yaml#
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,ipq9650-refgen-regulator
> +    then:
> +      required:
> +        - clocks
> +        - clock-names

Entire allOF block goes to the end, see example-schema.

> +
>  properties:
>    compatible:
>      oneOf:
> @@ -29,6 +39,7 @@ properties:
>  
>        - items:
>            - enum:
> +              - qcom,ipq9650-refgen-regulator
>                - qcom,qcs8300-refgen-regulator
>                - qcom,sa8775p-refgen-regulator
>                - qcom,sc7280-refgen-regulator
> @@ -45,6 +56,16 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  clocks:
> +    items:
> +      - description: Core reference clock
> +      - description: AHB interface clock
> +
> +  clock-names:
> +    items:
> +      - const: core
> +      - const: hclk

You just added clocks to each variant, which is not explained in commit
msg. And it would be a separate commit anyway. Probably you wanted to
add proper constraints (:false).


Best regards,
Krzysztof