Messages in this thread | | | Date | Wed, 22 May 2019 12:01:07 +0100 | From | Mark Brown <> | Subject | Re: [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 |
| |
On Tue, May 21, 2019 at 05:16:06PM -0600, Jeffrey Hugo wrote: > On 5/21/2019 12:50 PM, Mark Brown wrote:
> > > +static int spmi_regulator_common_list_voltage(struct regulator_dev *rdev, > > > + unsigned selector); > > > + > > > +static int spmi_regulator_common2_set_voltage(struct regulator_dev *rdev, > > > + unsigned selector)
> > Eeew, can we not have better names?
> I'm open to suggestions. Apparently there are two register common register > schemes - the old one and the new one. PMIC designs after some random point > in time are all the new register scheme per the documentation I see.
> As far as I an aware, the FT426 design is the first design to be added to > this driver to make use of the new scheme, but I expect more to be supported > in future, thus I'm reluctant to make these ft426 specific in the name.
If there's a completely new register map why are these even in the same driver?
> > > + if (reg == SPMI_COMMON2_MODE_HPM_MASK) > > > + return REGULATOR_MODE_NORMAL; > > > + > > > + if (reg == SPMI_COMMON2_MODE_AUTO_MASK) > > > + return REGULATOR_MODE_FAST; > > > + > > > + return REGULATOR_MODE_IDLE; > > > +}
> > This looks like you want to write a switch statement.
> It follows the existing style in the driver, but sure I can make this a > switch.
Please fix the rest of the driver as well then. [unhandled content-type:application/pgp-signature] | |