Re: [PATCH 13/14] regulator: max8925: fix not add device if missinginit data

[Haojian Zhuang]

On Thu, Apr 21, 2011 at 8:03 PM, Mark Brown
<broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/