linux-kernel - Re: [PATCH v4] regulator: MAX77686: Add Maxim 77686 regulator driver

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]

Date:	Tue, 29 May 2012 11:10:32 +0900
From:	jonghwa3.lee@...sung.com
To:	Yadwinder Singh Brar <yadi.brar01@...il.com>
Cc:	linux-kernel@...r.kernel.org, Liam Girdwood <lrg@...com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Chiwoong Byun <woong.byun@...sung.com>,
	Myungjoo Ham <myungjoo.ham@...sung.com>,
	Kyungmin Park <kyungmin.park@...sung.com>
Subject: Re: [PATCH v4] regulator: MAX77686: Add Maxim 77686 regulator driver

Hi, Yadwinder,

On 2012년 05월 25일 21:27, Yadwinder Singh Brar wrote:

> Hi Jonghwa,
> 
>> +
>> +       if (pdata)
>> +               max77686->opmode_data = pdata->opmode_data;
> 
> I think this is unused(unwanted) now.
> 


Sorry, i missed to remove it out. I'll remove it.

>> +
>> +       for (i = 0; i < MAX77686_REGULATORS; i++) {
>> +               if (pdata)
>> +                       init_data[pdata->regulators[i].id] =
>> +                                                pdata->regulators[i].initdata;
> 
> I  think we can directly use  pdata->regulators[i].initdata instead of
> init_data[i].
> In case if pdata is not their we can use same instance of
> init_data(default)  for all regulators.
> 


This if for some situation that pdata's initdata doensn't line up. When
user sets only initdata considered it being used, there may be
regulators not having initdata, also its order is not clear. So for
those state, i think just using temporary array which satisfies
regulator's id order is fine while it can't use pdata's initdata directly.

>> +
>> +               config.init_data = init_data[i];
>> +               rdev[i] = regulator_register(&regulators[i], &config);
>> +
> 
>> +
>> +       for (i = 0; i < 8; i++) {
>> +               if (pdata->buck2_voltage[i] > 0)
>> +                       ret = regulator_map_voltage_linear(
>> +                                       rdev[MAX77686_LDOS+1],
>> +                                       pdata->buck2_voltage[i],
>> +                                       pdata->buck2_voltage[i]
>> +                                       + MAX77686_DVS_UVSTEP);
>> +               /* 1.1V as default for safety */
>> +               if (pdata->buck2_voltage[i] <= 0 || ret < 0)
>> +                       max77686->buck2_vol[i] = 0x28;
>> +               else
>> +                       max77686->buck2_vol[i] = ret;
>> +               regmap_write(max77686->iodev->regmap,
>> +                        MAX77686_REG_BUCK2DVS1 + i, max77686->buck2_vol[i]);
>> +
>> +               if (pdata->buck3_voltage[i] > 0)
>> +                       ret = regulator_map_voltage_linear(
>> +                                       rdev[MAX77686_LDOS+1],
>> +                                       pdata->buck3_voltage[i],
>> +                                       pdata->buck3_voltage[i]
>> +                                       + MAX77686_DVS_UVSTEP);
>> +               /* 1.1V as default for safety */
>> +               if (pdata->buck3_voltage[i] <= 0 || ret < 0)
>> +                       max77686->buck3_vol[i] = 0x28;
>> +               else
>> +                       max77686->buck3_vol[i] = ret;
>> +               regmap_write(max77686->iodev->regmap,
>> +                        MAX77686_REG_BUCK3DVS1 + i, max77686->buck3_vol[i]);
>> +
>> +               if (pdata->buck4_voltage[i] > 0)
>> +                       ret = regulator_map_voltage_linear(
>> +                                       rdev[MAX77686_LDOS+1],
>> +                                       pdata->buck4_voltage[i],
>> +                                       pdata->buck4_voltage[i]
>> +                                       + MAX77686_DVS_UVSTEP);
>> +               /* 1.1V as default for safety */
>> +               if (pdata->buck4_voltage[i] <= 0 || ret < 0)
>> +                       max77686->buck4_vol[i] = 0x28;
>> +               else
>> +                       max77686->buck4_vol[i] = ret;
>> +               regmap_write(max77686->iodev->regmap,
>> +                        MAX77686_REG_BUCK4DVS1 + i, max77686->buck4_vol[i]);
>> +       }
> 
> Why do we need to initialize the 8 voltage registers of BUCK2/3/4  ?
> 


Yes you're right. It is useless because we don't support DVS mode at
this moment. I'll remove it either.

> 
> Regards,
> Yadwinder.
> --
> 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/
> 


Thanks ,
Best Regards.
--
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/