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@xxxxxxxxxxxxxxxx>
> ---
>  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