Re: [RFC PATCH v3 3/8] regulator: add pbias regulator support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 21, 2013 at 07:50:22PM +0530, Balaji T K wrote:

> +static int pbias_regulator_set_voltage(struct regulator_dev *dev,
> +			int min_uV, int max_uV, unsigned *selector)
> +{
> +	struct pbias_regulator_data *data = rdev_get_drvdata(dev);
> +	const struct pbias_bit_map *bmap = data->bmap;
> +	int ret, vmode;
> +
> +	if (min_uV <= 1800000)
> +		vmode = 0;
> +	else if (min_uV > 1800000)
> +		vmode = bmap->vmode;
> +
> +	ret = regmap_update_bits(data->syscon, data->pbias_reg,
> +						bmap->vmode, vmode);
> +	data->voltage = min_uV;

> +static int pbias_regulator_get_voltage(struct regulator_dev *rdev)
> +{
> +	struct pbias_regulator_data *data = rdev_get_drvdata(rdev);
> +
> +	return data->voltage;
> +}

These don't match up with each other - the get and set voltage calls
should reflect what the hardware state is, not what was requested by the
caller.  You should be able to use the regmap helpers I think.

> +static int pbias_regulator_enable(struct regulator_dev *rdev)
> +{
> +	struct pbias_regulator_data *data = rdev_get_drvdata(rdev);
> +	const struct pbias_bit_map *bmap = data->bmap;
> +	int ret;
> +
> +	ret = regmap_update_bits(data->syscon, data->pbias_reg,
> +					bmap->enable_mask, bmap->enable);

regulator_enable_regmap() and similarly for disable() and is_enabled().

> +	supply_name = initdata->constraints.name;
> +
> +	of_property_read_u32(np, "startup-delay-us", &startup_delay);
> +	ret = of_property_read_u32(np, "pbias-reg-offset",
> +				   &drvdata->pbias_reg);
> +	if (ret) {
> +		dev_err(&pdev->dev, "no pbias-reg-offset property set\n");
> +		return ret;
> +	}

This looks like it should be added as a standard property for overridig
the regulator delay if it can't be set based on the compatible string
alone due to board dependencies.  Do something like what's done for
regulator-ramp-delay.

> +err_regulator:
> +	kfree(drvdata->desc.name);
> +	return ret;

devm_kzalloc().

> +static int __init pbias_regulator_init(void)
> +{
> +	return platform_driver_register(&pbias_regulator_driver);
> +}
> +subsys_initcall(pbias_regulator_init);

module_platform_driver().

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux