[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 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

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

[Kathiravan Thirumoorthy]

On 6/4/2026 8:42 PM, Krzysztof Kozlowski wrote:
> 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.

Since the allOf already at the top of the file, I just added under it. 
Let me move it to the end.

>
>> +
>>   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).

Ack, will make the change to define the clock and clock-names only for 
IPQ9650 compatible under the allOf block.

>
>
> Best regards,
> Krzysztof

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

[Krzysztof Kozlowski]

On 05/06/2026 08:51, Kathiravan Thirumoorthy wrote:

>>>   
>>> +  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).
> 
> Ack, will make the change to define the clock and clock-names only for 
> IPQ9650 compatible under the allOf block.


No, That;s not what I said. That definition is fine, but you miss
constraints. See example-schema.


Best regards,
Krzysztof

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

[Kathiravan Thirumoorthy]

On 6/5/2026 1:33 PM, Krzysztof Kozlowski wrote:
> On 05/06/2026 08:51, Kathiravan Thirumoorthy wrote:
>
>>>>    
>>>> +  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).
>> Ack, will make the change to define the clock and clock-names only for
>> IPQ9650 compatible under the allOf block.
>
> No, That;s not what I said. That definition is fine, but you miss
> constraints. See example-schema.

Thanks.. Let me add the constraints with the else block.

>
>
> Best regards,
> Krzysztof

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

[Dmitry Baryshkov]

On Thu, Jun 04, 2026 at 05:07:35PM +0530, Kathiravan Thirumoorthy wrote:
> 
> 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.

Ack

> 
> 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",

-- 
With best wishes
Dmitry

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

[Konrad Dybcio]

On 6/2/26 11:22 AM, 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>
> ---

I'm slightly confused. Is there no register programming needed?
Can we at least retrieve the running state in .is_enabled() by reading
some value?

Konrad

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

[Kathiravan Thirumoorthy]

On 6/9/2026 7:18 PM, Konrad Dybcio wrote:
> On 6/2/26 11:22 AM, 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>
>> ---
> I'm slightly confused. Is there no register programming needed?

Yeah, no programming is needed. Similar to Kodiak, PHYs in IPQ9650 does 
the HW votes. No SW vote is needed.

> Can we at least retrieve the running state in .is_enabled() by reading
> some value?

Based on the discussion with IP team, I can use the REFGEN_STATUS 
register. Let me update the is_enabled() to query this information.

>
> Konrad