Messages in this thread | | | Date | Thu, 28 Jul 2022 12:11:08 +0100 | From | Mark Brown <> | Subject | Re: [PATCH v2 3/5] regulator: qcom_spmi: Add support for new regulator types |
| |
On Thu, Jul 28, 2022 at 02:14:10AM +0300, Iskren Chernev wrote: > 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).
This needs at least a comment.
> >> @@ -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.
This needs describing in the changelog, probably you need multiple paches here since you are making a number of different changes each of which needs some explanation.
> 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.
If that's the case perhaps part of this needs to be sent as a fix. [unhandled content-type:application/pgp-signature] | |