Re: [PATCH v5] regulator: Add GPIO enable control to fixed voltage regulator driver

From: Roger Quadros
Date: Wed Aug 05 2009 - 04:25:31 EST


On Tue, Aug 4, 2009 at 11:40 PM, Mark
Brown<broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, Aug 04, 2009 at 08:58:50PM +0300, Roger Quadros wrote:
>> From: Roger Quadros <ext-roger.quadros@xxxxxxxxx>
>
>> Now fixed regulators that have their enable pin connected to a GPIO line
>> can use the fixed regulator driver for regulator enable/disable control.
>> The GPIO number and polarity information is passed through platform data.
>> GPIO enable control is achieved using gpiolib.
>
>> Signed-off-by: Roger Quadros <ext-roger.quadros@xxxxxxxxx>
>
> Looks good, thanks for taking the time to do this and fixing up the
> review comments.
>
> Acked-by: Mark Brown <broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
>
> A couple of small notes that I don't see as blocking merge:
>
>> +
>> +             /* FIXME: Remove below print warning
>> +              *
>> +              * config->gpio must be set to -EINVAL by platform code if
>> +              * GPIO control is not required. However, early adopters
>> +              * not requiring GPIO control may forget to initialize
>> +              * config->gpio to -EINVAL. This will cause GPIO 0 to be used
>> +              * for GPIO control.
>> +              *
>> +              * This warning will be removed once there are a couple of users
>> +              * for this driver.
>
> Probably just for 2.6.33 or something; the point here is that this is an
> incompatible change in the platform data which we can get away with here
> due to the fact that there aren't any current users.
>
>> +             drvdata->is_enabled = config->enabled_at_boot;
>> +             ret = drvdata->is_enabled ?
>> +                             config->enable_high : !config->enable_high;
>
> Might be more legible without the ternery operator but no point spinning
> the patch for that.
>

Thank you all for review.

As reported by Liam, the patch does not apply on latest
git://git.kernel.org/pub/scm/linux/kernel/git/lrg/voltage-2.6.git#for-next
Seems it moved on in the last 2 days when from which I based my patch on.

I will regenerate the patch against latest HEAD and resend.

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