[UPSTREAM - RFC] pcf50633_use_regulator_api.patch
Balaji Rao
balaji at raobalaji.com
Wed Oct 8 17:21:48 CEST 2008
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.
>
Yes. Will do.
> > +#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?
>
Oh! I'm sorry about this. I forgot to change it. I had planned to do it
at runtime in the regulator_init function.
> > +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.
>
Ok, Will change.
> > +/*
> > + * 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
OK. Thank you for the comments, Mark.
- Balaji
More information about the openmoko-kernel
mailing list