On Wed, Mar 21, 2012 at 10:55:54AM -0700, Russ Dill wrote:

> unwieldy this would become if it were done for every device. Either the
> functionality needs to be moved to Snowball board init code, or a generic
> framework needs to be made for attaching regulators to arbitrary devices.

Hrm?  Adding regulator supply mappings anywhere other than the
initialisation for a specific board would be extremely unusual and
rather suspicious.

> +     supplies = kcalloc(ARRAY_SIZE(smsc911x_refs) * num,
> +                     sizeof(struct regulator_consumer_supply), GFP_KERNEL);
> +     if (!supplies) {
> +             pr_err("Failed to allocate memory\n");
> +             return;
> +     }
> +
> +     for (i = 0; i < ARRAY_SIZE(smsc911x_refs) * num; i++) {
> +             int id;
> +             char *name;
> +
> +             id = board_data[i / num].id;
> +             if (id != -1)
> +                     name = kasprintf(GFP_KERNEL, "smsc911x.%d", id);
> +             else
> +                     name = kstrdup("smsc911x", GFP_KERNEL);

This seems pretty much insane, it's costing a lot more to faff around
like this than it's worth.  Just do the setup in the individual boards,
if you really have no idea what's supplying the device (which seems a
bit unusual, more boards like this have things coming off the PMIC than
don't) there's now regulator_register_fixed() which cuts down on the
boilerplate a little.

I'd have complained about the original code if I'd noticed it wasn't a
patch for a particular board as the breakage you've found is obvious.
The regulation constraints it adds are bogus too, it's setting
REGULATOR_CHANGE_MODE on a regulator that doesn't support modes and
REGULATOR_CHANGE_STATUS without supplying the enable GPIO.

Attachment: signature.asc
Description: Digital signature

Reply via email to