linux-kernel - RE: [PATCH 13/14] regulator: max8925: fix not add device if missing init data

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: <25B60CDC2F704E4E9D88FFD52780CB4C05CF0EF027@SC-VEXCH1.marvell.com>
Date:	Thu, 21 Apr 2011 04:13:28 -0700
From:	Haojian Zhuang <hzhuang1@...vell.com>
To:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
CC:	"sameo@...ux.intel.com" <sameo@...ux.intel.com>,
	"haojian.zhuang@...il.com" <haojian.zhuang@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"lrg@...mlogic.co.uk" <lrg@...mlogic.co.uk>
Subject: RE: [PATCH 13/14] regulator: max8925: fix not add device if missing
 init data



>-----Original Message-----
>From: Mark Brown [mailto:broonie@...nsource.wolfsonmicro.com]
>Sent: 2011年4月21日 6:42 PM
>To: Haojian Zhuang
>Cc: sameo@...ux.intel.com; haojian.zhuang@...il.com; linux-
>kernel@...r.kernel.org; lrg@...mlogic.co.uk
>Subject: Re: [PATCH 13/14] regulator: max8925: fix not add device if
>missing init data
>
>On Wed, Apr 20, 2011 at 07:55:39PM +0800, Haojian Zhuang wrote:
>> If regulator[0] is missed in init data, all regulators of max8925
>won't
>> be initialized.
>
>With a changelog like this I'd expect a small change to an error check
>in the startup code or something but this is a very big change to the
>driver initialisation.
>
>> +static struct regulator_init_data
>regulator_pdata[ARRAY_SIZE(regulator_devs)];
>
>That looks really suspicious, what happens if there's two of these
>devices in the system?
>
It's impossible to use two PMIC in one system. At least, I didn't hear.

>> +		memcpy(&regulator_pdata[i], &pdata->regulator[i],
>> +			sizeof(struct regulator_init_data));
>> +		regulator_devs[i].platform_data = &regulator_pdata[i];
>> +		regulator_devs[i].pdata_size = sizeof(regulator_pdata[i]);
>> +		regulator_devs[i].num_resources = 1;
>> +		regulator_devs[i].resources = &regulator_resources[seq];
>> +
>> +		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[i], 1,
>> +				      &regulator_resources[seq], 0);
>> +		if (ret < 0) {
>> +			dev_err(chip->dev, "Failed to add regulator subdev\n");
>> +			goto out;
>
>It's really unclear why the array is needed at all if you're registering
>the devices one at a time.
>
Although there're a lot of regulators, maybe only parts of them are used.
So only add the necessary one. Others will be passed. The number is got
from platform data.

>> -	if (pdata && pdata->regulator[0]) {
>> -		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[0],
>> -				      ARRAY_SIZE(regulator_devs),
>> -				      &regulator_resources[0], 0);
>> -		if (ret < 0) {
>> -			dev_err(chip->dev, "Failed to add regulator subdev\n");
>> -			goto out_dev;
>> -		}
>
>Surely the only change that's needed here is to remove the check to see
>if pdata->regulator[0] is non-null?
All above changes are necessary to me.

Thanks
Haojian