Re: [PATCH v4 3/7] regulator: add pbias regulator support

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

 



On Tuesday 10 December 2013 04:10 PM, Mark Brown wrote:
On Tue, Dec 10, 2013 at 03:46:13PM +0530, Balaji T K wrote:

+config REGULATOR_PBIAS
+	tristate "PBIAS OMAP regulator driver"
+	depends on ARCH_OMAP && MFD_SYSCON

That should be (ARCH_OMAP || COMPILE_TEST) && MFD_SYSCON


Ok

+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_reg_info *info = data->info;
+	int ret, vmode;
+
+	if (min_uV <= 1800000)
+		vmode = 0;
+	else if (min_uV > 1800000)
+		vmode = info->vmode;
+
+	ret = regmap_update_bits(data->syscon, data->pbias_reg,
+						info->vmode, vmode);
+	data->voltage = min_uV;

This is exactly the same as it was the first time it was posted and is
still buggy.  To repeat the get and set voltage functions should reflect
the actual voltage set in the hardware.


my bad, will fix it.

+static int pbias_regulator_is_enable(struct regulator_dev *rdev)
+{
+	struct pbias_regulator_data *data = rdev_get_drvdata(rdev);
+	const struct pbias_reg_info *info = data->info;
+	int value;
+
+	regmap_read(data->syscon, data->pbias_reg, &value);
+
+	return value & info->enable_mask;
+}

If the enable mask really can have multiple bits this won't do the right
thing - it'll return true if any bits are set.  It needs to make sure
all the bits are set.


True, will fix it.

+#if CONFIG_OF

Why?

Driver is DT only, Will remove it.


+	drvdata->desc.n_voltages = 3;

This doesn't match your implementation which can only set two voltages.


I considered power-off too, will make it 2 then.

Thanks and Regards,
Balaji T K
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




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

  Powered by Linux