linux-kernel - Re: [PATCH 01/01] regulator: support max8649

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 25 Jan 2010 13:56:28 +0000
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Haojian Zhuang <haojian.zhuang@...il.com>
Cc:	Liam Girdwood <lrg@...mlogic.co.uk>,
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/01] regulator: support max8649

On Mon, Jan 25, 2010 at 06:01:41AM -0500, Haojian Zhuang wrote:
> On Tue, Jan 12, 2010 at 6:51 AM, Mark Brown

> >> +     if (pdata->ramp_timing) {
> >> +             info->ramp_timing = pdata->ramp_timing;
> >> +             max8649_set_bits(info->i2c, MAX8649_RAMP, MAX8649_RAMP_MASK,
> >> +                              info->ramp_timing << 5);
> >> +     }

> > You might want to implement the new enable_time() API for this.

> This ramp timing is the time interval of each step on adjusting
> voltage. I just want to control the timing in initialization.

If this applies to any voltage change at all then rather than just power
on I need to finish off the stuff I've been sitting on for handling slew
times for voltage changes.  If the regulator hasn't yet reached the
requested output when the consumer tries using it the consumer might get
broken.

If the ramp also gets applied when initially turning on the regulator
you'd still want to implement enable_time() for the same reason.

> enable_time() is only called before enabling regulator. And I don't
> understand what would be done in enable_time().

You'd return the amount of time taken to turn on the regulator and get
the output voltage stable in the current configuration.  This will then
be used by the core to ensure that the consumer only tries to use the
regulator once it's fully enabled.

> +static int max8649_set_mode(struct regulator_dev *rdev, unsigned int mode)
> +{
> +	struct max8649_regulator_info *info = rdev_get_drvdata(rdev);
> +
> +	switch (mode) {
> +	case REGULATOR_MODE_FAST:
> +	case REGULATOR_MODE_NORMAL:
> +		max8649_set_bits(info->i2c, info->vol_reg, MAX8649_FORCE_PWM,
> +				 MAX8649_FORCE_PWM);
> +		break;

Are you sure about this?  I'd expect to see force PWM used only for
_FAST, for _NORMAL pulse skipping is usually desired behaviour.

> +	case REGULATOR_MODE_IDLE:
> +	case REGULATOR_MODE_STANDBY:
> +		max8649_set_bits(info->i2c, info->vol_reg,
> +				 MAX8649_FORCE_PWM, 0);

I'd just leave these unimplemented (there's no need to support all
modes) and make sure that this ties in with...

> +static unsigned int max8649_get_mode(struct regulator_dev *rdev)
> +{
> +	struct max8649_regulator_info *info = rdev_get_drvdata(rdev);
> +	int ret;
> +
> +	ret = max8649_reg_read(info->i2c, info->vol_reg);
> +	if (ret & MAX8649_FORCE_PWM)
> +		return REGULATOR_MODE_NORMAL;
> +	return REGULATOR_MODE_IDLE;

...this.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/