Messages in this thread | | | Date | Thu, 21 Apr 2011 23:23:19 +0800 | Subject | Re: [PATCH 13/14] regulator: max8925: fix not add device if missing init data | From | Haojian Zhuang <> |
| |
On Thu, Apr 21, 2011 at 8:03 PM, Mark Brown <broonie@opensource.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, ®ulator_devs[0], >> >> - ARRAY_SIZE(regulator_devs), >> >> - ®ulator_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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |