On Sat, Aug 20, 2011 at 10:24:51PM +0200, Heiko Stübner wrote:
Mostly looks good, just a few fairly small comments below.
> + if (pdata->max_uA && pdata->max_uA > 500000
> + && max_uA >= pdata->max_uA) {
> + dev_dbg(rdev_get_dev(rdev),
> + "setting current limit to %d mA\n",
> + pdata->max_uA / 1000);
> + gpio_set_value(pdata->gpio_en2, 1);
> + gpio_set_value(pdata->gpio_en1, 0);
> + } else if (max_uA >= 500000) {
> + dev_dbg(rdev_get_dev(rdev),
> + "setting current limit to 500 mA\n");
> + gpio_set_value(pdata->gpio_en2, 0);
> + gpio_set_value(pdata->gpio_en1, 1);
> + } else if (max_uA >= 100000) {
> + dev_dbg(rdev_get_dev(rdev),
> + "setting current limit to 100 mA\n");
> + gpio_set_value(pdata->gpio_en2, 0);
> + gpio_set_value(pdata->gpio_en1, 0);
> + } else {
> + dev_dbg(rdev_get_dev(rdev),
> + "setting current limit to 0 mA\n");
> + gpio_set_value(pdata->gpio_en2, 1);
> + gpio_set_value(pdata->gpio_en1, 1);
> + }
I'd rather expect this to return an error sometimes.
> +static int bq2407x_get_current_limit(struct regulator_dev *rdev)
> +{
> + struct bq2407x_mach_info *pdata = rdev_get_drvdata(rdev);
> +
> + int en2 = gpio_get_value(pdata->gpio_en2);
> + int en1 = gpio_get_value(pdata->gpio_en1);
Calling gpio_get_value() on an output GPIO is undefined, you need to
remember what values you set.
> +static int bq2407x_enable(struct regulator_dev *rdev)
> +{
> + struct bq2407x_mach_info *pdata = rdev_get_drvdata(rdev);
> +
> + dev_dbg(rdev_get_dev(rdev), "enabling charger\n");
> +
> + gpio_set_value(pdata->gpio_nce, 0);
> + return 0;
Blank line here.
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
