linux-kernel - Re: [PATCH RFT 1/2] regulator: pcf50633: Don't write to reserved bits of AUTO output voltage select register

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAF+7xWmV71m4HR4wk-X1WxudNR5AFEuKpa=1wDa2Eo8krmLOcQ@mail.gmail.com>
Date:	Sat, 17 Mar 2012 07:55:00 +0800
From:	Axel Lin <axel.lin@...il.com>
To:	Lars-Peter Clausen <lars@...afoo.de>
Cc:	linux-kernel@...r.kernel.org, Liam Girdwood <lrg@...com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Subject: Re: [PATCH RFT 1/2] regulator: pcf50633: Don't write to reserved bits
 of AUTO output voltage select register

>> --- 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/