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

[Kathiravan Thirumoorthy]

On 6/15/2026 9:28 PM, Mark Brown wrote:
> On Mon, Jun 15, 2026 at 02:05:49PM +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, clock 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.
> > +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;
> > +
> > +	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);
> > +
> > +	return 0;
> > +}
> > +static const struct regulator_desc ipq9650_refgen_desc = {
> > +	.enable_reg = REFGEN_REG_REFGEN_STATUS,
> > +	.enable_mask = REFGEN_STATUS_OUT_MASK,
> > +	.enable_val = REFGEN_STATUS_OUT_ENABLE,
> > +	.ops = &(const struct regulator_ops) {
> > +		.enable		= qcom_ipq9650_refgen_enable,
> > +		.disable	= qcom_ipq9650_refgen_disable,
> > +		.is_enabled	= regulator_is_enabled_regmap,
> > +	},
> This looks like a get_status() operation, not an enable operation?  The
> enables and disables are pure clock operations.

Thanks, Mark for the review. If I understand correctly, I should track 
the clock enable/disable operations and return that state in the 
is_enabled() callback (as I did in v1). Please let me know if my 
understanding does not align with your expectations.