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: <BANLkTik3w-uv95jHdBt7Q7qU54zc1=gCFA@mail.gmail.com>
Date:	Thu, 21 Apr 2011 23:23:19 +0800
From:	Haojian Zhuang <haojian.zhuang@...il.com>
To:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Cc:	Haojian Zhuang <hzhuang1@...vell.com>,
	"sameo@...ux.intel.com" <sameo@...ux.intel.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

On Thu, Apr 21, 2011 at 8:03 PM, Mark Brown
<broonie@...nsource.wolfsonmicro.com> wrote:
> On Thu, Apr 21, 2011 at 04:13:28AM -0700, Haojian Zhuang wrote:
>
>> >> +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.
>
> I don't believe that.  It's not going to be a common case but it will be
> possible.
>
OK, I can allocate memory to handle this.

>> >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.
>
> You're missing the point here.  You're registering a device at a time
> but for some reason the platform data is being stored in an array in
> order to pass it in, even though only a single element in the array is
> in use.
>
I can allocate memory for 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.
>
> What makes you say this?  If you remove the check for the first entry
> being null then the driver will go and register all the regulators which
> seems fine.  That seems like it fixes the problem with skipping all the
> regulators if the first one has no platform data.
>
If I keep original method and only fix the null issue on regulator[0], I can
also register regulators. But it'll display error messages since
regulator_init_data is null while it's registers.

After switching to the current method, I only register the ones that I really
need. So I can avoid some error messages in boot time.

An array seems not good, and I can switch to allocate memory.

> You're really not explaining the changes you're trying to make here
> terribly well, the changes you're making seem very substantial relative
> to what you say you're trying to do.
>
Yes, I'll list all changes in log.
--
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/