Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1271568

Re: [PATCH V1] regulator: pv88060: new regulator driver

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

On Tue, Nov 17, 2015 at 11:25:21AM +0900, James Ban wrote:

A couple of minor points but overall this looks good:

> +	node = of_get_child_by_name(dev->of_node, "regulators");
> +	if (!node) {
> +		dev_err(dev, "regulators node not found\n");
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	num = of_regulator_match(dev, node, pv88060_matches,
> +				 ARRAY_SIZE(pv88060_matches));

Don't manually parse the DT, use the core support and set of_match and
regulators_node in the regulator_desc.

> +		if (i == 0) {
> +			pv88060_matches[i].init_data->constraints
> +				.valid_modes_mask
> +				|= REGULATOR_MODE_FAST
> +				| REGULATOR_MODE_NORMAL
> +				| REGULATOR_MODE_STANDBY;
> +			pv88060_matches[i].init_data->constraints.valid_ops_mask
> +				|=	REGULATOR_CHANGE_MODE;
> +		}

The driver should never modify the constraints that are passed in - the
whole point with constraints is to allow the system to set up what is
safe on that particular system.

> +	if (!chip->pdata)
> +		chip->pdata = pv88060_parse_regulators_dt(chip->dev);
> +
> +	if (IS_ERR(chip->pdata)) {
> +		dev_err(chip->dev, "No regulators defined for the platform\n");
> +		return PTR_ERR(chip->pdata);
> +	}

This should not be an error, the driver should just instantiate all
regulators the device has and at least let people read back the current
state.

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH V1] regulator: pv88060: new regulator driver James Ban <James.Ban.opensource@diasemi.com> - 2015-11-17 06:20 +0100
  Re: [PATCH V1] regulator: pv88060: new regulator driver Mark Brown <broonie@kernel.org> - 2015-11-17 20:00 +0100

csiph-web