lkml.org 
[lkml]   [2017]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] regulator: tps65132: add regulator driver for TI TPS65132
On Mon, Mar 06, 2017 at 10:06:58PM +0530, Venkat Reddy Talla wrote:

> + dis_mask = (id == TPS65132_REGULATOR_ID_VPOS) ?
> + TPS65132_REG_APPS_DISP : TPS65132_REG_APPS_DISN;

Please don't abuse the ternery operator, write normal if statements so
that people can read the code easily.

> +static int tps65132_disable(struct regulator_dev *rdev)
> +{
> + struct tps65132_regulator *tps = rdev_get_drvdata(rdev);
> + int id = rdev_get_id(rdev);
> + int act_dis_gpio = tps->reg_pdata[id].active_discharge_gpio;
> + unsigned int act_dis_time_us = tps->reg_pdata[id].active_discharge_time;
> +
> + if (gpio_is_valid(act_dis_gpio)) {
> + gpio_set_value_cansleep(act_dis_gpio, 1);
> + usleep_range(act_dis_time_us,
> + act_dis_time_us + TPS65132_ACT_DIS_TIME_SLACK);
> + gpio_set_value_cansleep(act_dis_gpio, 0);
> + }

Don't we need to undo the active discharge register write?

> +static int tps65132_get_regulator_dt_data(struct device *dev,
> + struct tps65132_regulator *tps65132_regs)
> +{
> + struct tps65132_reg_pdata *rpdata;
> + struct device_node *rnode;
> + int id;
> + int ret;
> +
> + ret = of_regulator_match(dev, dev->of_node, tps65132_regulator_matches,
> + ARRAY_SIZE(tps65132_regulator_matches));
> + if (ret < 0) {
> + dev_err(dev, "node parsing for regulator failed: %d\n", ret);
> + return ret;
> + }

Don't open code this, use of_parse_cb() to parse your extra properties.

> +static int __init tps65132_init(void)
> +{
> + return i2c_add_driver(&tps65132_i2c_driver);
> +}
> +subsys_initcall(tps65132_init);

You should use module_i2c_driver() for new drivers.
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2017-03-07 13:35    [W:0.040 / U:0.468 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site