Re: [PATCH RFT 1/2] regulator: pcf50633: Don't write to reserved bitsof AUTO output voltage select register

From: Axel Lin
Date: Fri Mar 16 2012 - 19:55:24 EST


>> --- a/drivers/regulator/pcf50633-regulator.c
>> +++ b/drivers/regulator/pcf50633-regulator.c
>> @@ -52,7 +52,7 @@ static const u8 pcf50633_regulator_registers[PCF50633_NUM_REGULATORS] = {
>>[...]
>>
>> @@ -161,6 +158,9 @@ static int pcf50633_regulator_voltage_value(enum pcf50633_regulator_id id,
>>
>>       switch (id) {
>>       case PCF50633_REGULATOR_AUTO:
>> +             /* AUTOOUT: 00000000 to 00101110 are reserved */
>> +             if (bits < 0x2f)
>> +                     return -EINVAL;
>>               millivolts = auto_voltage_value(bits);
>>               break;
>>       case PCF50633_REGULATOR_DOWN1:
>> @@ -208,20 +208,7 @@ static int pcf50633_regulator_get_voltage(struct regulator_dev *rdev)
>>  static int pcf50633_regulator_list_voltage(struct regulator_dev *rdev,
>>                                               unsigned int index)
>>  {
>> -     struct pcf50633 *pcf;
>> -     int regulator_id;
>> -
>> -     pcf = rdev_get_drvdata(rdev);
>> -
>> -     regulator_id = rdev_get_id(rdev);
>> -
>> -     switch (regulator_id) {
>> -     case PCF50633_REGULATOR_AUTO:
>> -             index += 0x2f;
>> -             break;
>> -     default:
>> -             break;
>> -     }
>
> Does this make sense? Now we return -EINVAL if index is less than 0x2f
>
>
I check the code and regulator core again.
I think for this case we should return 0 instead of -EINVAL.

/**
* regulator_list_voltage - enumerate supported voltages
* @regulator: regulator source
* @selector: identify voltage to list
* Context: can sleep
*
* Returns a voltage that can be passed to @regulator_set_voltage(),
* zero if this selector code can't be used on this system, or a
* negative errno.
*/

I'll send a V2 soon.
Thanks for the review.
Axel
--
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/