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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Device Tree ABI breakage and regression in driver probing for older DTBs.
--

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

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

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.

> diff --git a/Documentation/devicetree/bindings/clock/qcom,glymur-tcsr.yaml b/Documentation/devicetree/bindings/clock/qcom,glymur-tcsr.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,glymur-tcsr.yaml

[ ... ]

> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,glymur-tcsr
> +    then:
> +      required:
> +        - vdda-qrefrpt0-0p9-supply

[Severity: High]
Does making these new regulator supplies unconditionally required cause a
regression for Device Tree ABI compatibility?

Older device trees lacking these properties will fail schema validation.

[ ... ]

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

[ ... ]

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

[ ... ]

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

[Severity: High]
Since older device trees won't have these regulator properties defined, will
this unconditional devm_regulator_bulk_get() cause probe to fail entirely?

If full constraints are enforced without globally enabled dummy regulators,
devm_regulator_bulk_get() will return -ENODEV and cause a regression where the
TCSR clock controller fails to probe for systems booting with an older DTB.

> +		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602-tcsr_qref_0527-v5-0-8ea174a59d7e@oss.qualcomm.com?part=5