[UPSTREAM - RFC] pcf50633_use_regulator_api.patch

[Balaji Rao]

On Wed, 8 Oct 2008 15:57:54 +0100
Mark Brown <broonie at opensource.wolfsonmicro.com> wrote:

> On Wed, Oct 08, 2008 at 07:38:45PM +0530, Balaji Rao wrote:
> 
> The regulator bits of this look pretty good.  A few comments below...
> 
> > @@ -0,0 +1,259 @@
> > +/*
> > + * Regulator driver for pcf50633
> > + */
> > +
> > +#include <linux/regulator/driver.h>
> > +#include <linux/pcf50633.h>
> 
> Should probably push that into linux/mfd.
> 
> > +#define PCF50633_REGULATOR(_name, _id) \
> > +	{				\
> > +		.name = _name, .id = _id, .ops =
> > &pcf50633_reg_ops, \
> > +		.irq = 1, .type = REGULATOR_VOLTAGE, \
> > +		.owner = THIS_MODULE, \
> > +	}
> 
> The IRQ ought to be passed in as board/platform data rather than being
> hard coded.  Though I can't currently see any actual use of it
> currently so it'd be even easier to just drop it for now?
> 
> > +static int pcf50633_regulator_set_voltage(struct regulator_dev
> > *rdev,
> > +			int min_uV, int max_uV)
> > +{
> 
> ...
> 
> > +	millivolts = min_uV / 1000;
> > +
> > +	regnr = regulator_registers[regulator_id];
> > +
> > +	switch (regulator_id) {
> > +	case PCF50633_REGULATOR_AUTO:
> > +		volt_bits = auto_voltage_bits(millivolts);
> > +		break;
> 
> There's no check to see if the voltage being requested is actually
> supported by the regulator - if the regulator can't satisfy the
> request it should return -EINVAL.
> 
> > +/* 
> > + * Initialize and register the regulator with the core.
> > + */
> > +
> > +int pcf50633_regulator_init(struct pcf50633_data *pcf, int irq)
> > +{
> > +	int i;
> > +
> > +	printk("pcf50633 regulator init\n");
> > +
> > +	for (i = 0; i < ARRAY_SIZE(pcf50633_regulators); i++) {
> > +		pcf50633_regulators[i].irq = irq;
> > +		regulator_register(&pcf50633_regulators[i], pcf);
> > +	}
> > +
> > +	return 0;
> > +}
> 
> This has changed in Liam's current for-linus branch.  See
> 
> 	git://git.kernel.org/pub/scm/linux/kernel/git/lrg/voltage-2.6

Ah! It looks to have changed quite a bit!, which means there is more
reading to do. :-)

	- Balaji