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 09:50:50 +0530
From:	Yadwinder Singh Brar <yadi.brar01@...il.com>
To:	jonghwa3.lee@...sung.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

On Tue, May 29, 2012 at 7:40 AM,  <jonghwa3.lee@...sung.com> wrote:
 Hi Jonghwa,

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

Ok, but I think this not right place for sorting (sorting is not taking
place.) You have to sort it before entering in loop for registering
regulators.

> user sets only initdata considered it being used, there may be
> regulators not having initdata, also its order is not clear. So for

Ok, I think this is a bug in present driver also, because
without checking pdata->num_regulators, you are running in
loop  for (i = 0; i < MAX77686_REGULATORS; i++)
where MAX77686_REGULATORS should be equal to
pdata->num_regulators for this driver to work fine.

If we consider a case pdata->num_regulators is
equal to MAX77686_REGULATORS and initdata is
not their(i.e. NULL) than I think it will initialise
init_data[pdata->regulators[i].id to NULL, which again will be a bug.

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

If I am not wrong, I think we can also sort pdata's initdata also using
kernel's sort api and use one instance of (default)initdata for
all unused or uninitialized regulators in platform file.

>>> +
>>> +               config.init_data = init_data[i];
>>> +               rdev[i] = regulator_register(&regulators[i], &config);
>>> +
>>

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/