On Thu, Nov 05, 2015 at 09:34:47PM +0800, Chen Feng wrote:
> +config REGULATOR_HI6220_MTCMOS
> + bool "Hisilicon Hi6220 mtcmos support"
> + depends on ARCH_HISI
> + help
> + This driver provides support for the mtcmos regulators of Hi6220 Soc.
> +
The Kconfig and Makefile updates for MCTMOS should have been in the
patch adding the driver for that.
> +config REGULATOR_HI655X
> + bool "HiSilicon Hi655x PMIC voltage regulator support"
> + depends on ARCH_HISI
For both of these we should have an || COMPILE_TEST and there's no need
for either to be bool I can see, they should be tristate.
> +static int hi655x_is_enabled(struct regulator_dev *rdev)
> +{
> + unsigned int value = 0;
> +
> + struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
> + struct hi655x_regulator_ctrl_regs *ctrl_regs = ®ulator->ctrl_regs;
> +
> + regmap_read(rdev->regmap, ctrl_regs->status_reg, &value);
> + return (value & BIT(regulator->ctrl_mask));
> +}
Use the standard regmap helpers, don't open code them.
> +static int hi655x_set_voltage(struct regulator_dev *rdev,
> + int min_uV, int max_uV, unsigned *selector)
Use the standard helpers, including one of the map_voltage()s and
set_voltage_sel_regmap(), don't open code them.
> +static unsigned int hi655x_map_mode(unsigned int mode)
> +{
> + /* hi655x pmic on hi6220 SoC only support normal mode */
> + if (mode == REGULATOR_MODE_NORMAL)
> + return REGULATOR_MODE_NORMAL;
> + else
> + return -EINVAL;
> +}
If the device only has one mode it should not have any mode operations,
they're only meaningful if there are multiple modes to set and they are
optional.
Attachment:
signature.asc
Description: PGP signature
