Message-ID: <751e5129-3c11-0156-719e-3fe996a149be@quicinc.com>
Date:   Tue, 7 Mar 2023 12:25:04 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
From:   Devi Priya <quic_devipriy@quicinc.com>
Subject: Re: [PATCH V2 4/6] regulator: qcom_smd: Add support to define the
 bootup voltage
To:     Konrad Dybcio <konrad.dybcio@linaro.org>,
        Mark Brown <broonie@kernel.org>
CC:     <agross@kernel.org>, <andersson@kernel.org>, <lgirdwood@gmail.com>,
        <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <quic_srichara@quicinc.com>,
        <quic_gokulsri@quicinc.com>, <quic_sjaganat@quicinc.com>,
        <quic_kathirav@quicinc.com>, <quic_arajkuma@quicinc.com>,
        <quic_anusha@quicinc.com>, <quic_ipkumar@quicinc.com>
References: <20230217142030.16012-1-quic_devipriy@quicinc.com>
 <20230217142030.16012-5-quic_devipriy@quicinc.com>
 <907628d1-b88d-5ac6-ed9d-7f63e2875738@linaro.org>
 <Y/aeu5ua7cY5cGON@sirena.org.uk>
 <39f73580-f263-de0e-6819-89c3f4c75c3a@quicinc.com>
 <8ce07abd-2d02-69d2-8dc6-fe11525aecda@linaro.org>
 <11b05b9f-b969-6648-2204-2da5f8465c96@quicinc.com>
Content-Language: en-US
In-Reply-To: <11b05b9f-b969-6648-2204-2da5f8465c96@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 3/6/2023 6:39 PM, Devi Priya wrote:
> 
> 
> On 3/3/2023 6:57 PM, Konrad Dybcio wrote:
>>
>>
>> On 3.03.2023 14:21, Devi Priya wrote:
>>>
>>>
>>> On 2/23/2023 4:31 AM, Mark Brown wrote:
>>>> On Wed, Feb 22, 2023 at 11:11:42PM +0100, Konrad Dybcio wrote:
>>>>
>>>>> Thinking about it again, this seems like something that could be
>>>>> generalized and introduced into regulator core.. Hardcoding this
>>>>> will not end well.. Not to mention it'll affect all mp5496-using
>>>>> boards that are already upstream.
>>>>
>>>>> WDYT about regulator-init-microvolts Mark?
>>>>
>>>> The overwhelming majority of devices that have variable voltages
>>>> support readback, these Qualcomm firmware devices are pretty much
>>>> unique in this regard.  We don't want a general property to set a
>>>> specific voltage since normally we should be using the
>>>> constraints and don't normally need to adjust things immediately
>>>> since we can tell what the current voltage is.
>>>>
>>>> This is pretty much just going to be a device specific bodge,
>>>> ideally something that does know what the voltage is would be
>>>> able to tell us at runtime but if that's not possible then
>>>> there's no good options.  If the initial voltage might vary based
>>>> on board then a device specific DT property might be less
>>>> terrible, if it's determined by the regulator the current code
>>>> seems fine.  Or just leave the current behavour, if the
>>>> constraints are accurate then hopefully a temporary dip in
>>>> voltage is just inelegant rather than an issue.  Indeed the
>>>> current behaviour might well save power if you've got a voltage
>>>> range configured and nothing actually ever gets round to setting
>>>> the voltage (which is depressingly common, people seem keen on
>>>> setting voltage ranges even when the voltage is never varied in
>>>> practice).
>>>
>>> Hi Mark, The initial bootup voltage is actually blown into the OTP 
>>> register of the PMIC and it remains the same across boards for 
>>> IPQ9574 SoC.
>> But what about IPQ6018 which also uses MP5496? That's also gonna
>> set the voltage on there, it may be too high/low..
For IPQ6018, the bootup voltage is the same as that of IPQ9574 which is
875mV
>>
>>   Initially the SoC runs at 800MHz with a voltage of 875mV set by the 
>> bootloaders. As kernel does not know the initial voltage, during 
>> regulator registration the framework considers the current voltage to 
>> be zero and tries to bring up the regulator to minimum supported 
>> voltage of 600mV. This causes the dip which might be of concern in SS 
>> parts where the voltage might be insufficient leading to silent reboots.
>> That's an SoC-specific thing, the same regulator can be used with
>> many different ones. We can't just assume it'll always be like this.
>> I see the problem, but I believe this is not the correct solution
Okay, As we had discussions on reading back the voltage & the generic
DT property, do you suggest any other possible solutions here?
>>
>> Konrad
>>>
>>> Best Regards,
>>> Devi Priya