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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `qcom,ipq9650-refgen-regulator` compatible string incorrectly mandates a fallback to `qcom,sm8250-refgen-regulator`, which violates device tree backward compatibility contracts and causes a kernel panic on older kernels.
--

commit 0fcc5b0e92deea2c90fae28f9e7822774bfdb4e3
Author: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>

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

This commit documents the IPQ9650 compatible string in the refgen-regulator
binding. It also specifies that IPQ9650 requires explicitly enabled REFGEN
clocks, unlike other supported platforms.

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

[ ... ]

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

[Severity: High]
Does grouping qcom,ipq9650-refgen-regulator here mandate a fallback to
qcom,sm8250-refgen-regulator?

Since IPQ9650 requires explicit clock enablement, an older kernel using the
fallback string won't enable these clocks. Furthermore, IPQ9650 uses a
different control register compared to the fallback (0xC vs 0x80).

If an older kernel probes using the qcom,sm8250-refgen-regulator fallback,
the unmodified driver will attempt to read the 0x80 register during
registration:

drivers/regulator/qcom-refgen-regulator.c:qcom_refgen_probe() {
    ...
    rdev = devm_regulator_register(dev, rdesc, &config);
    ...
}

Could this unclocked MMIO access trigger a synchronous external abort
(SError) and crash the system during boot? If the hardware requires a
different programming sequence and clocks, should it avoid using the
SM8250 fallback entirely to avoid regressions on older kernels?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611-ipq9650_refgen-v2-0-d96a91d5b99e@oss.qualcomm.com?part=2