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: <20110421120327.GD11788@opensource.wolfsonmicro.com>
Date:	Thu, 21 Apr 2011 13:03:28 +0100
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Haojian Zhuang <hzhuang1@...vell.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

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.

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

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

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ