Re: [PATCH V3] regulator: DA9211 : new regulator driver

[Mark Brown]

On Mon, Jun 02, 2014 at 08:17:03AM +0100, James Ban wrote:

> This is the driver for the Dialog DA9211 Multi-phase 12A DC-DC Buck
> Converter regulator. It communicates via an I2C bus to the device.

This *still* seems to have all the same problems with inconsistent
handling of the A and B register sets 

> +	/* Make sure to exit from suspend mode on enable */
> +	if (regulator->reg_rselect == DA9211_RSEL_NO_GPIO) {
> +		ret = regmap_update_bits(chip->regmap, info->conf.reg,
> +					info->conf.sel_mask,
> +					DA9211_VBUCKA_SEL_A);
> +		if (ret < 0)
> +			return ret;
> +	}

Here it forces set A if and only if in no GPIO mode but...

> +static int da9211_regulator_set_suspend_voltage(struct regulator_dev *rdev,
> +						int uV)
> +{
> +	struct da9211_regulator *regulator = rdev_get_drvdata(rdev);
> +	struct da9211_regulator_info *info = regulator->info;
> +	struct da9211 *chip = regulator->da9211;
> +	int ret;
> +
> +	ret = regulator_map_voltage_linear(rdev, uV, uV);
> +	if (ret < 0)
> +		return ret;
> +
> +	return regmap_update_bits(chip->regmap, info->volt.reg_b,
> +				 info->volt.v_mask, ret);
> +}

...this unconditionally uses set B and...

> +static int da9211_suspend_enable(struct regulator_dev *rdev)
> +{
> +	struct da9211_regulator *regulator = rdev_get_drvdata(rdev);
> +	struct da9211_regulator_info *info = regulator->info;
> +	struct da9211 *chip = regulator->da9211;
> +
> +	/* Select register set B for voltage ramping. */
> +	if (regulator->reg_rselect == DA9211_RSEL_NO_GPIO)
> +		return regmap_update_bits(chip->regmap, info->conf.reg,
> +					info->conf.sel_mask,
> +					DA9211_VBUCKA_SEL_B);
> +	else
> +		return 0;
> +}

...this uses B if not using a GPIO and otherwise reports success without
actually doing anything (which seems buggy).  These behaviours aren't
compatible with each other, set_suspend_voltage() will break if set B is
selected by GPIO for example.

Please either go through and try to make these things all consistent or
(probably better) strip out the suspend and probably GPIO handling for
now, get the bulk of the driver merged, and then come back and try to
deal with those tricky bits.

Attachment: signature.asc
Description: Digital signature