linux-kernel - Re: [PATCH v2 2/2] MAX8997/8966 PMIC Regulator Driver Initial Release

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: <AANLkTik4cT3fbsCLFcsUcNMtia1Ymnhe8JFzzO+N+9sq@mail.gmail.com>
Date:	Tue, 8 Mar 2011 10:50:04 +0900
From:	MyungJoo Ham <myungjoo.ham@...sung.com>
To:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Cc:	linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
	Liam Girdwood <lrg@...mlogic.co.uk>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	kyungmin.park@...sung.com, MyungJoo Ham <myungjoo.ham@...il.com>
Subject: Re: [PATCH v2 2/2] MAX8997/8966 PMIC Regulator Driver Initial Release

On Sat, Mar 5, 2011 at 9:03 PM, Mark Brown
<broonie@...nsource.wolfsonmicro.com> wrote:
> On Fri, Mar 04, 2011 at 03:50:27PM +0900, MyungJoo Ham wrote:

[]

>
>> +static int max8997_reg_enable_suspend(struct regulator_dev *rdev)
>> +{
>> +     return 0;
>> +}
>> +
>
> This looks odd, especially since you have a disable operation?

The intention is to keep it enabled if it was enabled before entering
sleep and not to enable if it has not been using while the system is
running. Probably, we need three states for suspend-prepare for
regulators: disable, enable, keep_state?

>

[]

>
>> +static int max8997_resume(struct device *dev)
>> +{
>> +     struct platform_device *pdev = container_of(dev,
>> +                     struct platform_device, dev);
>> +     struct max8997_data *max8997 = platform_get_drvdata(pdev);
>> +     struct i2c_client *i2c = max8997->iodev->i2c;
>> +     struct regulator_dev *rdev;
>> +     int ret, reg, mask, pattern;
>> +     int i;
>> +     u8 val;
>> +
>> +     /* Show Error/Warning Messages for Inconsistency */
>> +     for (i = 0; i < MAX8997_REG_MAX; i++) {
>> +             if (max8997->saved_rdev[i]) {
>
> We should put this into the regulator core rather than doing it in
> drivers - it's not really driver specific and other regulators that
> can't preserve state over suspend and resume will need it.
>

This part was sort of "debug and test" part of the driver and removed
in the recent version of the patch.
However, the regulator core depends on calling
regulator_suspend_prepare by machine/architecture pm code and the core
does not have resume-related support. For that matter, I've made and
been using "regulator_suspend_finish" locally. I will post that patch
soon.



Thanks.


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