linux-kernel - Re: [PATCH v2 3/5] regulator: qcom_spmi: Add support for new regulator types

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <79077e08-4bd8-6967-748d-876589ef978e@gmail.com>
Date:   Thu, 28 Jul 2022 02:14:10 +0300
From:   Iskren Chernev <iskren.chernev@...il.com>
To:     Mark Brown <broonie@...nel.org>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        Adam Skladowski <a39.skl@...il.com>,
        Andy Gross <agross@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Robert Marko <robimarko@...il.com>,
        Jorge Ramirez <jorge.ramirez-ortiz@...aro.org>,
        devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org, phone-devel@...r.kernel.org,
        ~postmarketos/upstreaming@...ts.sr.ht
Subject: Re: [PATCH v2 3/5] regulator: qcom_spmi: Add support for new
 regulator types




On 7/27/22 14:57, Mark Brown wrote:
> On Tue, Jul 26, 2022 at 09:11:31PM +0300, Iskren Chernev wrote:
>
>> Add support for some regulator types that are missing in this driver, all
>> belonging to the FTSMPS426 register layout.  This is done in preparation
>> for adding support for the PM6125 PMIC.
>
>> +	.set_mode		= spmi_regulator_ftsmps3_set_mode,
>> +	.get_mode		= spmi_regulator_ftsmps426_get_mode,
>
> Why are set and get asymmetric?

Because the get method, only uses AUTO and HPM, which have the same value
for ftsmps3 and ftsmps426 (so there is no need for a new function).

>> @@ -1473,7 +1557,7 @@ static const struct spmi_regulator_mapping supported_regulators[] = {
>>  	SPMI_VREG(LDO,   HT_P600,  0, INF, HFS430, hfs430, ht_p600, 10000),
>>  	SPMI_VREG(LDO,   HT_P150,  0, INF, HFS430, hfs430, ht_p150, 10000),
>>  	SPMI_VREG(BUCK,  GP_CTL,   0, INF, SMPS,   smps,   smps,   100000),
>> -	SPMI_VREG(BUCK,  HFS430,   0, INF, HFS430, hfs430, hfs430,  10000),
>> +	SPMI_VREG(BUCK,  HFS430,   0,   3, HFS430, hfs430, hfs430,  10000),
>
> The changelog said we were adding support for new types but this looks
> like changing an existing type.

The code, as written now does a different thing for BUCK, HFS430 (on
mainline (ML) and downstream (DS) linked in the commit message). Since DS
only supports newer stuff, to be on safe side, I kept existing behavior for
rev 0-3 on BUCK(3)+HFS430(10), so at least DS and ML agree on pm6125
completely.

The commit [1] that adds support for BUCK+HFS430 might be wrong, or it
might be right for the time being (i.e initial revisions had different
behavior). I'm CC-ing Jorge.

Question is is BUCK+HFS430 on common2 (ftsmps426) or common3 (ftsmps3) or
a mix (depending on revision).

[1] 0211f68e626f (regulator: qcom_spmi: add PMS405 SPMI regulator, 2019-06-17)