linux-kernel - Re: [PATCH v4 2/3] regulator MAX8998/LP3974: Support DVS-GPIO.

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTi=dJ83par+U8YfbpC3HVjEkc9rfPU8pxSFN_mTb@mail.gmail.com>
Date:	Tue, 4 Jan 2011 17:16:56 +0900
From:	MyungJoo Ham <myungjoo.ham@...sung.com>
To:	Lukasz Majewski <l.majewski@...sung.com>
Cc:	linux-kernel@...r.kernel.org, Samuel Ortiz <sameo@...ux.intel.com>,
	Liam Girdwood <lrg@...mlogic.co.uk>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Alessandro Zummo <a.zummo@...ertech.it>,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Joonyoung Shim <jy0922.shim@...sung.com>
Subject: Re: [PATCH v4 2/3] regulator MAX8998/LP3974: Support DVS-GPIO.

On Tue, Jan 4, 2011 at 4:49 PM, Lukasz Majewski <l.majewski@...sung.com> wrote:
> On Tue, 04 Jan 2011 14:17:40 +0900
> MyungJoo Ham <myungjoo.ham@...sung.com> wrote:
>
> Hi all,
>
> I've posted some comments below:
>
> if (gpio_is_valid(pdata->buck2_set3)) {
>> -                     if (max8998->buck2_vol[0] == i) {
>> -                             max8998->buck2_idx = 0;
>> -                             buck2_gpio_set(pdata->buck2_set3, 0);
>> -                     } else {
>> -                             max8998->buck2_idx = 1;
>> -                             ret =
>> max8998_get_voltage_register(rdev, &reg, -
>> &shift,
>> -
>> &mask);
>> -                             ret = max8998_write_reg(i2c, reg, i);
>> -                             max8998->buck2_vol[1] = i;
>> -                             buck2_gpio_set(pdata->buck2_set3, 1);
>> +
>> +                     /* check if requested voltage */
>> +                     /* value is already defined */
>> +                     for (j = 0; j <
>> ARRAY_SIZE(max8998->buck2_vol); j++) {
>> +                             if (max8998->buck2_vol[j] == i) {
>> +                                     max8998->buck2_idx = j;
>> +
>> buck2_gpio_set(pdata->buck2_set3, j);
>> +                                     goto buck2_exit;
>> +                             }
>>                       }
>> +
>> +                     if (pdata->buck_voltage_lock)
>> +                             return -EINVAL;
>> +
>> +                     max8998_get_voltage_register(rdev,
>> +                                     &reg, &shift, &mask);
>> +                     ret = max8998_write_reg(i2c, reg, i);
>> +                     max8998->buck2_vol[max8998->buck2_idx] = i;
>> +
>> buck2_gpio_set(pdata->buck2_set3,max8998->buck2_idx); +buck2_exit:
>> dev_dbg(max8998->dev, "%s: SET3:%d\n", i2c->name,
> gpio_get_value(pdata->buck2_set3));
>
> Maybe only the matter of taste. The "for" loop for only two elements?

It was only for the look; to appear more symmetric with buck1.

>
>> + * @buck_voltage_lock: Do NOT change the values of the following six
>> + *   registers set by buck?_voltage?. The voltage of BUCK1/2 cannot
>> + *   be other than the preset values.
>> + * @buck1_voltage1: BUCK1 DVS mode 1 voltage register
>> + * @buck1_voltage2: BUCK1 DVS mode 2 voltage register
>> + * @buck1_voltage3: BUCK1 DVS mode 3 voltage register
>> + * @buck1_voltage4: BUCK1 DVS mode 4 voltage register
>> + * @buck2_voltage1: BUCK2 DVS mode 1 voltage register
>> + * @buck2_voltage2: BUCK2 DVS mode 2 voltage register
>
> Is it desirable to define all four for BUCK1 and two for BUCK2 DVS
> voltages in platform code?

In case a system with 4 buck1 preset voltages and 2 buck2 preset
voltages, it is desirable. :)

>
> Why the "general purpose" slots approach for user changeable/definable
> voltages (as it was done previously) have been replaced? Is the current
> approach faster?

As long as "buck_voltage_lock" is not set, user may use voltages not
defined as a preset (buckx_voltagex).

If buck_voltage_lock is true, any voltage not predefined is
rejected.However, if not, undefined voltages are accepted and from the
point where an undefined voltage is submitted, presets are not
guaranteed to be kept (any of preset values may be overwritten.)

>
>
> --
> Best regards,
>
> Lukasz Majewski
>
> Samsung Poland R&D Center
> Platform Group
>

Thanks.

-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
--
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/