On Tue, Apr 15, 2014 at 01:14:36PM -0700, Doug Anderson wrote: > Mitigate the problem by: > * Allow setting the overcurrent wait time so devices with this problem > can set it to the max. > * Add retry logic on enables of FETs.
This is two changes, should really be two patches.
> +- ti,overcurrent-wait: This is applicable to FET registers, which have a
> + poorly defined "overcurrent wait" field. If this property is present it
> + should be between 0 - 3. If this property isn't present we won't touch the
> + "overcurrent wait" field and we'll leave it to the BIOS/EC to deal with.
I take it this is the raw value to write to the register?
> +static int tps65090_fet_is_enabled(struct regulator_dev *rdev)
> +{
> + unsigned int control;
> + unsigned int expected = rdev->desc->enable_mask | BIT(CTRL_PG_BIT);
> + int ret;
> +
> + ret = regmap_read(rdev->regmap, rdev->desc->enable_reg, &control);
> + if (ret != 0)
> + return ret;
> +
> + return (control & expected) == expected;
> +}
No need to open code this, regulator_is_enabled_regmap() can check for
any value in a bitfield.
> +static int tps6090_try_enable_fet(struct regulator_dev *rdev)
Why is this called tps6090_try_enable_fet(), looks like a missing 5?
> + /*
> + * Try enabling multiple times until we succeed since sometimes the
> + * first try times out.
> + */
> + for (tries = 0; ; tries++) {
> + ret = tps6090_try_enable_fet(rdev);
> + if (!ret)
> + break;
> + if (ret != -ENOTRECOVERABLE || tries == MAX_FET_ENABLE_TRIES)
> + goto err;
Make this a do { } while so we don't have the exit condition missing in
the for loop please, it's doing the right thing but it's not as obvious
as it could be.
> + if (tries) {
> + dev_warn(&rdev->dev, "reg %#x enable ok after %d tries\n",
> + rdev->desc->enable_reg, tries);
> + }
No need for braces here, and I guess that ought to be retries rather
than tries (though that is pedantry).
signature.asc
Description: Digital signature
