[PATCH] regulator: core: add regulator_get_linear_step()

[PATCH] regulator: core: add regulator_get_linear_step()

[Paul Walmsley]

Add regulator_get_linear_step(), which returns the voltage step size
between VSEL values for linear regulators.  This is intended for use
by regulator consumers which build their own voltage-to-VSEL tables.

Signed-off-by: Paul Walmsley <pwalmsley@nvidia.com>
Reviewed-by: Andrew Chew <achew@nvidia.com>
Cc: Matthew Longnecker <mlongnecker@nvidia.com>
---

Applies on v3.10-rc4.  Will be used by the upcoming Tegra DFLL clocksource 
driver, which will build its own table of voltage-to-VSEL values by 
querying the regulator framework.

 drivers/regulator/core.c           |   15 +++++++++++++++
 include/linux/regulator/consumer.h |    1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 6e50178..f07d7ad 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2135,6 +2135,21 @@ int regulator_list_voltage(struct regulator *regulator, unsigned selector)
 EXPORT_SYMBOL_GPL(regulator_list_voltage);
 
 /**
+ * regulator_get_linear_step - return the voltage step size between VSEL values
+ * @regulator: regulator source
+ *
+ * Returns the voltage step size between VSEL values for linear
+ * regulators, or return 0 if the regulator isn't a linear regulator.
+ */
+unsigned int regulator_get_linear_step(struct regulator *regulator)
+{
+	struct regulator_dev *rdev = regulator->rdev;
+
+	return rdev->desc->uV_step;
+}
+EXPORT_SYMBOL_GPL(regulator_get_linear_step);
+
+/**
  * regulator_is_supported_voltage - check if a voltage range can be supported
  *
  * @regulator: Regulator to check.
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 145022a..3a76389c 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -165,6 +165,7 @@ int regulator_count_voltages(struct regulator *regulator);
 int regulator_list_voltage(struct regulator *regulator, unsigned selector);
 int regulator_is_supported_voltage(struct regulator *regulator,
 				   int min_uV, int max_uV);
+unsigned int regulator_get_linear_step(struct regulator *regulator);
 int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV);
 int regulator_set_voltage_time(struct regulator *regulator,
 			       int old_uV, int new_uV);
-- 
1.7.10.4

Re: [PATCH] regulator: core: add regulator_get_linear_step()

[Mark Brown]

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

On Fri, Jun 07, 2013 at 08:06:56AM +0000, Paul Walmsley wrote:

> Applies on v3.10-rc4.  Will be used by the upcoming Tegra DFLL clocksource 
> driver, which will build its own table of voltage-to-VSEL values by 
> querying the regulator framework.

Can you show the code please?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] regulator: core: add regulator_get_linear_step()

[Paul Walmsley]

Hi Mark,

On Fri, 7 Jun 2013, Mark Brown wrote:

> On Fri, Jun 07, 2013 at 08:06:56AM +0000, Paul Walmsley wrote:
> 
> > Applies on v3.10-rc4.  Will be used by the upcoming Tegra DFLL clocksource 
> > driver, which will build its own table of voltage-to-VSEL values by 
> > querying the regulator framework.
> 
> Can you show the code please?

The driver isn't 100% ready to post yet. Still wrapping up a few last 
tweaks (in an unrelated part of the driver).  But here are the parts of 
the code that use that regulator function.

This is the immediate caller:

+/**
+ * tegra_dfll_init_pmic_data - initialize PMIC regulator data
+ * @pdev: DFLL instance
+ *
+ * Read the PMIC integration data, including regulator data, from DT
+ * and the the regulator framework.  Build the voltage map from
+ * regulator data.  Returns 0 upon success or -EINVAL upon error.
+ */
+static int __must_check tegra_dfll_init_pmic_data(struct platform_device *pdev)
+{
+	struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
+	int r;
+
+	td->vdd = regulator_get(&pdev->dev, "vdd");
+	if (IS_ERR(td->vdd)) {
+		dev_err(&pdev->dev, "couldn't locate regulator\n");
+		return -EINVAL;
+	}
+
+	td->vdd_step = regulator_get_linear_step(td->vdd);
+	if (!td->vdd_step) {
+		dev_err(&pdev->dev, "only linear map regulators supported\n");
+		goto tdipd_err;
+	}
+
+	r = parse_of_dfll_pmic_integration(pdev, pdev->dev.of_node);
+	if (r) {
+		dev_err(&pdev->dev, "DFLL PMIC integration parse error\n");
+		goto tdipd_err;
+	}
+
+	r = tegra_dfll_regulator_probe_voltages(pdev);
+	if (r) {
+		dev_err(&pdev->dev, "couldn't probe regulator voltages\n");
+		goto tdipd_err;
+	}
+
+	return 0;
+
+tdipd_err:
+	regulator_put(td->vdd);
+	return -EINVAL;
+}


... and here's the code that uses td->vdd_step:

+static int get_cvb_voltage(struct platform_device *pdev, int c0, int c1,
+			   int c2)
+{
+	struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
+	/* apply only speedo scale: output mv = cvb_mv * v_scale */
+	int mv;
+
+	/* combined: apply voltage scale and round to cvb alignment step */
+	mv = DIV_ROUND_CLOSEST(c2 * td->speedo_value, td->cvb_speedo_scale);
+	mv = DIV_ROUND_CLOSEST((mv + c1) * td->speedo_value,
+			       td->cvb_speedo_scale) + c0;
+
+	/* XXX Bring back cvb_alignment_uv; put it in the board dfll DT */
+	return DIV_ROUND_UP(mv * 1000,
+			    td->cvb_voltage_scale * td->vdd_step) *
+		td->vdd_step / 1000;
+}


But, maybe you're more interested in the voltage probing code:


+/**
+ * tegra_dfll_regulator_probe_voltages - build vdd_map[] from the regulator
+ * @pdev: DFLL instance
+ *
+ * Build the vdd_map from regulator framework and DFLL DT data.
+ * Returns 0 upon success, or -ENOSPC on a memory allocation failure.
+ */
+static int tegra_dfll_regulator_probe_voltages(struct platform_device *pdev)
+{
+	struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
+	int c, i, j, vdd_uv, vdd_mv;
+	struct tegra_dfll_voltage_reg_map *vdd_map;
+
+	c = regulator_count_voltages(td->vdd);
+	if (c < 0)
+		return c;
+
+	vdd_map = kzalloc(sizeof(struct tegra_dfll_voltage_reg_map) * c,
+			  GFP_KERNEL);
+	if (!vdd_map)
+		return -ENOMEM;
+
+	j = 0;
+	for (i = 0; i < c; i++) {
+		vdd_uv = regulator_list_voltage(td->vdd, i);
+		if (vdd_uv <= 0)
+			continue;
+
+		if (vdd_uv < td->dfll_min_microvolt)
+			continue;
+
+		if (vdd_uv > td->dfll_max_microvolt)
+			break;
+
+		vdd_mv = vdd_uv / 1000;
+
+		vdd_map[j].reg_value = i;
+		vdd_map[j].reg_uv = vdd_uv;
+		vdd_map[j].reg_mv = vdd_mv;
+		j++;
+	}
+
+	td->vdd_map_size = j;
+	td->vdd_map = vdd_map;
+
+	return 0;
+}


If you've got a different approach in mind, I'm happy to switch to it.


- Paul

Re: [PATCH] regulator: core: add regulator_get_linear_step()

[Mark Brown]

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

On Fri, Jun 07, 2013 at 09:38:10AM +0000, Paul Walmsley wrote:

> +static int get_cvb_voltage(struct platform_device *pdev, int c0, int c1,
> +			   int c2)
> +{
> +	struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
> +	/* apply only speedo scale: output mv = cvb_mv * v_scale */
> +	int mv;
> +
> +	/* combined: apply voltage scale and round to cvb alignment step */
> +	mv = DIV_ROUND_CLOSEST(c2 * td->speedo_value, td->cvb_speedo_scale);
> +	mv = DIV_ROUND_CLOSEST((mv + c1) * td->speedo_value,
> +			       td->cvb_speedo_scale) + c0;
> +
> +	/* XXX Bring back cvb_alignment_uv; put it in the board dfll DT */
> +	return DIV_ROUND_UP(mv * 1000,
> +			    td->cvb_voltage_scale * td->vdd_step) *
> +		td->vdd_step / 1000;
> +}

Hrm, right.  So I guess for this the question is why you need the steps
at all - can't the driver just ask for the voltage it wants with
whatever tolerance is appropriate (or just specify the maximum tolerable
voltage if that's better)?  I'm not sure what cvb_voltage_scale is.

I guess the problem here is that you have to hit some fairly tight
tolerances on the voltage?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] regulator: core: add regulator_get_linear_step()

[Paul Walmsley]

Hi Mark,

On Fri, 7 Jun 2013, Mark Brown wrote:

> On Fri, Jun 07, 2013 at 09:38:10AM +0000, Paul Walmsley wrote:
> 
> > +static int get_cvb_voltage(struct platform_device *pdev, int c0, int c1,
> > +			   int c2)
> > +{
> > +	struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
> > +	/* apply only speedo scale: output mv = cvb_mv * v_scale */
> > +	int mv;
> > +
> > +	/* combined: apply voltage scale and round to cvb alignment step */
> > +	mv = DIV_ROUND_CLOSEST(c2 * td->speedo_value, td->cvb_speedo_scale);
> > +	mv = DIV_ROUND_CLOSEST((mv + c1) * td->speedo_value,
> > +			       td->cvb_speedo_scale) + c0;
> > +
> > +	/* XXX Bring back cvb_alignment_uv; put it in the board dfll DT */
> > +	return DIV_ROUND_UP(mv * 1000,
> > +			    td->cvb_voltage_scale * td->vdd_step) *
> > +		td->vdd_step / 1000;
> > +}
> 
> Hrm, right.  So I guess for this the question is why you need the steps
> at all - can't the driver just ask for the voltage it wants with
> whatever tolerance is appropriate (or just specify the maximum tolerable
> voltage if that's better)?  I'm not sure what cvb_voltage_scale is.
> 
> I guess the problem here is that you have to hit some fairly tight
> tolerances on the voltage?

The IP block has an I2C controller embedded in it that autonomously sends 
set-voltage commands (across a range of voltages) to the PMIC.  To enable 
that, the driver must first initialize the IP block with a 
voltage-to-selector table.


- Paul

Re: [PATCH] regulator: core: add regulator_get_linear_step()

[Paul Walmsley]

On Fri, 7 Jun 2013, Paul Walmsley wrote:

> The IP block has an I2C controller embedded in it that autonomously sends 
> set-voltage commands (across a range of voltages) to the PMIC.  To enable 
> that, the driver must first initialize the IP block with a 
> voltage-to-selector table.

Here's the table loading code -

+/**
+ * _load_lut - load voltage lookup table into DFLL RAM
+ * @pdev: DFLL instance
+ *
+ * Load the voltage-to-PMIC register value lookup table into the DFLL
+ * IP block LUT memory.  td->lut_min and td->lut_max are used to cap
+ * the minimum and maximum voltage requested.  This function shouldn't
+ * be called directly by code other than dfll_load_lut(), since this
+ * function doesn't handle the necessary pre- and post-requisites.  No
+ * return value.
+ */
+static void _load_lut(struct platform_device *pdev)
+{
+	struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
+	int i;
+	u32 val;
+
+	val = td->out_map[td->lut_min]->reg_value;
+	for (i = 0; i <= td->lut_min; i++)
+		dfll_writel(td, val, DFLL_OUTPUT_LUT + i * 4);
+
+	for (; i < td->lut_max; i++) {
+		val = td->out_map[i]->reg_value;
+		dfll_writel(td, val, DFLL_OUTPUT_LUT + i * 4);
+	}
+
+	val = td->out_map[td->lut_max]->reg_value;
+	for (; i < td->num_voltages; i++)
+		dfll_writel(td, val, DFLL_OUTPUT_LUT + i * 4);
+
+	dfll_wmb(td);
+}



- Paul

Re: [PATCH] regulator: core: add regulator_get_linear_step()

[Mark Brown]

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

On Fri, Jun 07, 2013 at 10:03:22AM +0000, Paul Walmsley wrote:

> The IP block has an I2C controller embedded in it that autonomously sends 
> set-voltage commands (across a range of voltages) to the PMIC.  To enable 
> that, the driver must first initialize the IP block with a 
> voltage-to-selector table.

Ah, right - a one of those!  OK, makes sense, I'll apply.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]