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

[Axel Lin]

>> --- 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/