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)
