On Mon, Apr 28, 2014 at 10:58:26AM +0530, Keerthy wrote:

> index 6a79328..5ddb220 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -384,6 +384,18 @@ config REGULATOR_PALMAS
>         on the muxing. This is handled automatically in the driver by
>         reading the mux info from OTP.
>  
> +config REGULATOR_TPS65917
> +     tristate "TI TPS65917 PMIC Regulators"
> +     depends on MFD_TPS65917

Please keep the Kconfig and Makefile sorted.

> +static const struct regs_info tps65917_regs_info[] = {
> +     {
> +             .name           = "SMPS1",
> +             .sname          = "smps1-in",
> +             .vsel_addr      = TPS65917_SMPS1_VOLTAGE,
> +             .ctrl_addr      = TPS65917_SMPS1_CTRL,
> +             .sleep_id       = TPS65917_EXTERNAL_REQSTR_ID_SMPS1,
> +     },

> +static int tps65917_is_enabled_smps(struct regulator_dev *dev)
> +{
> +     struct tps65917_pmic *pmic = rdev_get_drvdata(dev);
> +     int id = rdev_get_id(dev);
> +     unsigned int reg;
> +
> +     tps65917_smps_read(pmic->tps65917,
> +                        tps65917_regs_info[id].ctrl_addr, &reg);
> +
> +     reg &= TPS65917_SMPS1_CTRL_STATUS_MASK;
> +     reg >>= TPS65917_SMPS1_CTRL_STATUS_SHIFT;
> +
> +     return !!(reg);
> +}

A lot of these functions look like they could be implemented using the
standard helpers rather than open coded - the helpers now support
multi-bit enable fields.

> +     if (selector == 0)
> +             return 0;
> +     else if (selector < 6)
> +             return 500000 * mult;
> +     else
> +             /* Voltage is linear mapping starting from selector 6,
> +              * volt = (0.49V + ((selector - 5) * 0.01V)) * RANGE
> +              * RANGE is either x1 or x2
> +              */
> +             return (490000 + ((selector - 5) * 10000)) * mult;
> +}

This looks like the driver could just use linear ranges.

> +static struct of_device_id of_tps65917_match_tbl[] = {
> +     { .compatible = "ti,tps65917-pmic", },
> +};

Missing null entry at the end of the table here.

Attachment: signature.asc
Description: Digital signature

Reply via email to