Re: [PATCH 3/4] arm64: dts: qcom: Add Shikra CQM SoM platform

[Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>]

On 13/05/2026 17:29, Rakesh Kota wrote:
> On Wed, May 13, 2026 at 03:01:47PM +0300, Dmitry Baryshkov wrote:
>> On Wed, May 13, 2026 at 04:28:35AM +0000, sashiko-bot@kernel.org wrote:
>>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>> - [High] The PMIC regulator definitions omit their required input supply dependencies (e.g., `vdd_s2-supply`, `vdd_l3-supply`), breaking the power hierarchy.
>>> - [Medium] The device tree inaccurately hardcodes the `compatible` string to a different PMIC model (`qcom,rpm-pm2250-regulators`) instead of explicitly identifying the actual hardware (PM4125).
>>> --
>>>
>>> commit 457abf6e7ac410430c866842e08e602ec3daaa51
>>> Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
>>>
>>> arm64: dts: qcom: Add Shikra CQM SoM platform
>>>
>>> Add device tree include for the CQM variant of the Shikra System-on-Module.
>>> This variant integrates the Shikra SoC and PM4125 PMIC. The DTSI includes
>>> the common shikra.dtsi and adds PM4125 PMIC regulator definitions.
>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
>>>> new file mode 100644
>>>> index 0000000000000..401e71720519d
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
>>> [ ... ]
>>>> +&rpm_requests {
>>>> +	regulators {
>>>> +		compatible = "qcom,rpm-pm2250-regulators";
>>>
>>> Since the commit message indicates this is a PM4125 PMIC, should the
>>> compatible string include a specific identifier for the actual hardware?
>>>
>>> Using a specific identifier like "qcom,rpm-pm4125-regulators" alongside
>>> the fallback allows the kernel to apply specific quirks if PM4125 errata
>>> are discovered later.
>>
>> This can be ignored. The compat is a leftover from the historically
>> incorrect naming.
>>
>>>
>>>> +
>>>> +		pm4125_s2: s2 {
>>>> +			regulator-min-microvolt = <1000000>;
>>>> +			regulator-max-microvolt = <1200000>;
>>>> +		};
>>>
>>> Do these regulators need to explicitly define their input supply dependencies
>>> such as vdd_s2-supply?
>>>
>>> Without these properties, the regulator framework might be unaware that the
>>> PMIC regulators draw power from upstream supplies.
>>>
>>> If the kernel dynamically manages the upstream supply and its reference count
>>> drops to zero, could it be disabled, causing an unexpected power loss for
>>> downstream components?
>>
>> And this is a correct comment. Please provide missing supplies.
>>
> As per the Qualcomm system design, the parent-child supply relationship
> is managed by the RPM firmware, not the Linux regulator framework. The
> RPM ensures the parent supply is never disabled until all subsystem
> votes are cleared.

How is this different from other, previous platforms?

-- 
With best wishes
Dmitry