Messages in this thread | | | Date | Fri, 21 Jun 2013 16:24:16 +0100 | From | Mark Brown <> | Subject | Re: [PATCH V2] regulator: 88pm800: add regulator driver for 88pm800 |
| |
On Thu, Jun 20, 2013 at 03:31:07AM -0400, Chao Xie wrote:
Looks pretty good, main thing here is that some of the functionality here appears to be reproducing standard code.
> +static int pm800_set_voltage(struct regulator_dev *rdev, > + int min_uv, int max_uv, unsigned *selector) > +{
Use set_voltage_sel() and map_voltge_ascend()
> +static int pm800_get_voltage(struct regulator_dev *rdev) > +{ > +
Just provide get_voltage_sel(), the core will do the mapping to voltages using list_voltage().
> + } else if (pdata->num_regulators) { > + /* Check whether num_regulator is valid. */ > + unsigned int count = 0; > + for (i = 0; pdata->regulators[i]; i++) > + count++; > + if (count != pdata->num_regulators) > + return -EINVAL;
This looks... odd.
> +static int __init pm800_regulator_init(void) > +{ > + return platform_driver_register(&pm800_regulator_driver); > +} > +subsys_initcall(pm800_regulator_init); > + > +static void __exit pm800_regulator_exit(void) > +{ > + platform_driver_unregister(&pm800_regulator_driver); > +} > +module_exit(pm800_regulator_exit);
With deferred probing you should just be able to use module_platform_driver().
> +config REGULATOR_88PM800 > + bool "Marvell 88PM800 Power regulators" > + depends on MFD_88PM800=y > + help > + This driver supports Marvell 88PM800 voltage regulator chips. > + It delivers digitally programmable output, > + the voltage is programmed via I2C interface. > + It's suitable to support PXA988 chips to control VCC_MAIN and > + various voltages. > +
Keep this file sorted please.
> --- a/drivers/regulator/Makefile > +++ b/drivers/regulator/Makefile > @@ -70,6 +70,7 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o > obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o > obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o > obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o > +obj-$(CONFIG_REGULATOR_88PM800) += 88pm800.o
Same here. [unhandled content-type:application/pgp-signature] | |