Re: [PATCH] [RFC] Correct registration of multiple gpmc smsc911x devices.

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

[-- Attachment #1: Type: text/plain, Size: 1608 bytes --]

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 #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]