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
