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.
signature.asc
Description: Digital signature
